Skip to content

Conversation

grod220
Copy link
Member

@grod220 grod220 commented Apr 4, 2025

The javascript companion to the CLI equivalent functionality: #44. This allows for Wraps to be done in a multisig setup.

The chief workflow is:

  • All multisig holders run the multisigOfflineSignWrap() function and pass the result to the final broadcaster. The payer must also execute this function.
  • All results are passed to multisigBroadcastWrap() where it is validated, combined together, and broadcast to the network.

Contains an example that executes these steps.

@grod220 grod220 force-pushed the js-client-wrap-multisig branch from 7714dab to 69a4867 Compare April 4, 2025 15:31
@grod220 grod220 marked this pull request as ready for review April 4, 2025 15:48
@grod220 grod220 requested a review from mcintyre94 April 4, 2025 15:48
@grod220 grod220 force-pushed the js-client-wrap-multisig branch from 69a4867 to b2aa4e3 Compare April 7, 2025 07:28
Copy link
Member

@mcintyre94 mcintyre94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some comments, a lot of them are just style things that I think will make it a bit easier to maintain this.

But overall I think these helpers are doing a little too much, and should just be focused on returning a transaction to the caller. I'd cut out all the code that sends/confirms transactions, and let the caller re-use whatever logic they're using elsewhere in their app.

I might not be completely understanding the expected use case here, but since this is part of a @solana-program lib I think it likely makes sense for it not to be concerned with things like sending transactions.

@grod220 grod220 force-pushed the js-client-wrap-multisig branch from b2aa4e3 to 13f7d9a Compare April 16, 2025 12:53
@grod220
Copy link
Member Author

grod220 commented Apr 16, 2025

@mcintyre94 very helpful review! Implemented your feedback. Think it makes lots of sense.

@grod220 grod220 requested a review from mcintyre94 April 16, 2025 13:00
mcintyre94
mcintyre94 previously approved these changes Apr 22, 2025
Copy link
Member

@mcintyre94 mcintyre94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I think this is the right interface now and it all looks good. The only thing I'd suggest is getting rid of the arrow functions, because we don't usually use them in our libraries or the generated clients. And making sure all the functions have a return type.

@grod220 grod220 force-pushed the js-client-wrap-multisig branch from a740634 to 63fb236 Compare April 23, 2025 07:57
@grod220 grod220 merged commit 58df064 into main Apr 23, 2025
10 checks passed
@grod220 grod220 deleted the js-client-wrap-multisig branch April 23, 2025 10:19
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.

2 participants