Skip to content

Conversation

@d10r
Copy link
Collaborator

@d10r d10r commented Dec 16, 2024

What & Why

The CFA hook afterAgreementTerminated is invoked when a flow to the SuperApp or a flow from the SuperApp is deleted.
The latter case is unexpected for many, because it exists only for the delete callback, not for the create and update callbacks.
For SuperApps creating outgoing flows and implementing this hook without being aware of it (e.g. not checking the sender argument), this creates the risk of unexpected states, unexpected state updates, and of jailing.

How

  • invoke onFlowDeleted only for deletion of incoming flows and remove the receiver argument. This breaks existing code using it, which we want in order to make sure the change can't cause unnoticed behaviour change of such contracts.
  • invoke a new hook onOutFlowDeleted for deletion of outgoing flows. This allows to specifically handle this case, e.g. to implement sticky streams (don't allow closing by reopening in the hook).

Note: the change is breaking in the sense that code using it and updating the dependency needs to be changed.
Being just a library which is not used by the protocol itself, it's not breaking in the sense that deployed contracts change behaviour.

@github-actions
Copy link

github-actions bot commented Dec 16, 2024

Changelog Reminder

Reminder to update the CHANGELOG.md for any of the modified packages in this PR.

  • CHANGELOG.md modified
  • Double check before merge

@d10r d10r force-pushed the cfa_hook_no_footgun branch from 9fd6502 to 5519053 Compare January 2, 2025 14:48
@d10r d10r changed the title [ETHEREUM-CONTRACTS] CFASuperAppBase: explicit hook for deleted outflow [ETHEREUM-CONTRACTS] replace CFASuperAppBase with SuperAppBase Jan 2, 2025
@d10r d10r changed the title [ETHEREUM-CONTRACTS] replace CFASuperAppBase with SuperAppBase [ETHEREUM-CONTRACTS] make CFASuperAppBase safer Jan 2, 2025
@d10r d10r changed the title [ETHEREUM-CONTRACTS] make CFASuperAppBase safer [ETHEREUM-CONTRACTS] remove footgun from CFASuperAppBase Jan 2, 2025
@d10r d10r changed the base branch from dev-next to dev January 2, 2025 16:35
@d10r d10r marked this pull request as ready for review January 2, 2025 19:47
@d10r d10r requested a review from hellwolf as a code owner January 2, 2025 19:47
@hellwolf hellwolf marked this pull request as draft March 12, 2025 10:19
@d10r d10r changed the base branch from dev to 2025-10-zerotransfers October 22, 2025 11:29
@d10r d10r changed the base branch from 2025-10-zerotransfers to dev October 22, 2025 11:37
@d10r d10r marked this pull request as ready for review October 22, 2025 12:16
@hellwolf
Copy link
Contributor

@d10r can you check if this can address #2069 ?

@d10r
Copy link
Collaborator Author

d10r commented Oct 23, 2025

@d10r can you check if this can address #2069 ?

can do. But that makes the change considerably more breaking. Worth it?

@hellwolf
Copy link
Contributor

@d10r can you check if this can address #2069 ?

can do. But that makes the change considerably more breaking. Worth it?

It seems so.

@d10r d10r merged commit 9f2c5fe into dev Oct 24, 2025
22 checks passed
@d10r d10r deleted the cfa_hook_no_footgun branch October 24, 2025 12:34
@github-actions
Copy link

XKCD Comic Relif

Link: https://xkcd.com/2043
https://xkcd.com/2043

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants