Skip to content

feat: Add afterAdd hook to transaction controller#5692

Merged
matthewwalsh0 merged 4 commits intomainfrom
feat/after-add-hook
Apr 24, 2025
Merged

feat: Add afterAdd hook to transaction controller#5692
matthewwalsh0 merged 4 commits intomainfrom
feat/after-add-hook

Conversation

@matthewwalsh0
Copy link
Member

Explanation

Add optional afterAdd hook to mutate transactions added via addTransaction method.

Persist original transaction params in new txParamsOriginal property.

References

Fixes #4688

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review April 22, 2025 20:10
@matthewwalsh0 matthewwalsh0 requested review from a team as code owners April 22, 2025 20:10
@jpuri
Copy link
Contributor

jpuri commented Apr 23, 2025

Hey @matthewwalsh0 : I have a query about these hooks approach, but this can result in code written in clients and also replicated across extension / mobile.

@matthewwalsh0
Copy link
Member Author

matthewwalsh0 commented Apr 23, 2025

Hey @matthewwalsh0 : I have a query about these hooks approach, but this can result in code written in clients and also replicated across extension / mobile.

From the perspective of the controllers, they're just injection points for custom logic so we can decouple from other domains and client specific decisions such as smart transactions, delegations, and snaps.

But if the same hook is needed in both clients, there is nothing stopping the hook being defined within another controller or central package, and the hook just invoking that via the messenger, as we should indeed avoid duplication where possible.

@matthewwalsh0 matthewwalsh0 enabled auto-merge (squash) April 24, 2025 08:49
@matthewwalsh0 matthewwalsh0 merged commit 9852a3c into main Apr 24, 2025
202 checks passed
@matthewwalsh0 matthewwalsh0 deleted the feat/after-add-hook branch April 24, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants