Skip to content

solana: Remove *_multisig instruction variants#593

Merged
nik-suri merged 14 commits intomainfrom
solana/remove-multisig-variants
Mar 25, 2025
Merged

solana: Remove *_multisig instruction variants#593
nik-suri merged 14 commits intomainfrom
solana/remove-multisig-variants

Conversation

@nvsriram
Copy link
Copy Markdown
Contributor

@nvsriram nvsriram commented Mar 5, 2025

This PR adds:

  • Remove initialize_multisig and release_inbound_mint_multisig ix
  • Add Option<'info, SplMultisig<'info>> account to Initialize and ReleaseInboundMint struct
  • Update cargo and TS tests
  • Derive multisig token authority from the token mint authority
  • Update IDL

@nvsriram
Copy link
Copy Markdown
Contributor Author

nvsriram commented Mar 5, 2025

As this PR introduces breaking changes to initialize and release_inbound_mint, this PR also covers breaking changes to release_inbound_* - introduced by #336.

@nvsriram nvsriram marked this pull request as ready for review March 5, 2025 22:08
@nonergodic
Copy link
Copy Markdown
Contributor

nonergodic commented Mar 6, 2025

Btw, I like the streamlining of integrating the multisig handling in the respective instructions instead of doing the ix multiplexing 👍

nvsriram and others added 14 commits March 6, 2025 11:02
There are a few suspicious aspects of the inbox release logic:
1. `release_inbound_unlock` takes a flag that determines whether a
   `false` result from `try_release` should cause an error. The flag
   indicates that it should revert on delay. However, `try_release` can
   also return `false` when a transaction has not been approved. If the
   flag is set, the transaction will not return an error when a user
   tries to release a transaction that is not approved even though
   whether a transaction is approved has nothing to do with its expected
   behaviour when it is delayed
2. The custom error TransferNotApproved is defined but unused

Taken together, I believe `try_release()` should revert with the
TransferNotApproved error. This would disambiguate the various 'false'
results and provide more intuitive behaviour when a user tries to
release a transaction that is not approved.
@nvsriram nvsriram force-pushed the solana/remove-multisig-variants branch from 060499f to cc573c3 Compare March 6, 2025 16:02
Copy link
Copy Markdown
Contributor

@nik-suri nik-suri left a comment

Choose a reason for hiding this comment

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

Stamping this PR to unblock merge. 2nd review will come via security review by Asymmetric Research.

@nik-suri nik-suri merged commit 5f2882e into main Mar 25, 2025
9 checks passed
@nik-suri nik-suri deleted the solana/remove-multisig-variants branch March 25, 2025 15:21
SC4RECOIN added a commit to m0-foundation/solana-m that referenced this pull request Jul 29, 2025
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.

4 participants