fix(token): mark InitializeMultisig member accounts as read-only - #461
fix(token): mark InitializeMultisig member accounts as read-only#461ozpool wants to merge 1 commit into
Conversation
SPL Token InitializeMultisig/InitializeMultisig2 declare the member
(signer) accounts as read-only non-signer ([]); the program records
their pubkeys into the multisig data and does not require them to sign
the setup transaction. AddSigners was tagging each member .SIGNER(),
in both programs/token and programs/token-2022, contradicting the
struct's own doc ("requires no signers"). Drop the signer flag and
sync the account-annotation headers. Adds TestAccountFlags_* asserting
members are read-only non-signer and the multisig account stays
writable.
Closes solana-foundation#460
Greptile SummaryThis PR corrects a spurious
Confidence Score: 5/5Safe to merge — the change corrects a mismatch between the builder output and the on-chain SPL Token account spec, and the new tests directly guard the fixed behaviour. Each of the four affected builders gets a single, targeted removal of the incorrect No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller
participant Builder as InstructionBuilder
participant Instruction as InitializeMultisig(2)
participant Chain as Solana Runtime
Caller->>Builder: NewInitializeMultisig(2)InstructionBuilder()
Builder->>Builder: SetAccount(multisigPubkey) → [WRITE]
Builder->>Builder: AddSigners(member1, member2, ...) → [] (read-only, non-signer)
Builder->>Instruction: Build()
Instruction->>Chain: Submit transaction
Note over Chain: Program reads member pubkeys<br/>from accounts array and<br/>stores them in multisig data.<br/>No member signatures required.
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Caller
participant Builder as InstructionBuilder
participant Instruction as InitializeMultisig(2)
participant Chain as Solana Runtime
Caller->>Builder: NewInitializeMultisig(2)InstructionBuilder()
Builder->>Builder: SetAccount(multisigPubkey) → [WRITE]
Builder->>Builder: AddSigners(member1, member2, ...) → [] (read-only, non-signer)
Builder->>Instruction: Build()
Instruction->>Chain: Submit transaction
Note over Chain: Program reads member pubkeys<br/>from accounts array and<br/>stores them in multisig data.<br/>No member signatures required.
Reviews (1): Last reviewed commit: "fix(token): mark InitializeMultisig memb..." | Re-trigger Greptile |
|
@jacobcreech apologies for the cold tag — you've merged here before, so hoping you can point me in the right direction rather than me guessing. This one's been open since July 3 with green CI and no human review yet, and I'm not sure who's picking up SDK reviews at the moment. Not chasing a merge — just want to make sure it's in someone's queue rather than quietly lost. Short version: Entirely happy to close it if the existing flags are deliberate for a reason I've missed — #460 has the full write-up. |
What
InitializeMultisigandInitializeMultisig2, in bothprograms/tokenandprograms/token-2022, tagged each multisig member account with the signer flag:The canonical SPL Token account spec declares those accounts read-only, non-signer:
The program only records the member pubkeys into the multisig account data; it does not require them to sign the
InitializeMultisigtransaction. The struct's own doc comment already states this ("The InitializeMultisig instruction requires no signers ..."), so the builder contradicted its own documentation.Fix
Drop the spurious
.SIGNER()inAddSignersfor both instructions in both programs, and sync the[2...] = [SIGNER]/[1] = [SIGNER]annotation headers to[]. One-flag change per builder; the multisig account itself stays[WRITE], the rent sysvar stays read-only.Tests
Adds
TestAccountFlags_InitializeMultisig/TestAccountFlags_InitializeMultisig2to both packages, asserting the members are read-only non-signer and the multisig account is writable. Proven to fail on the current code (member account 0 must not be a signer) and pass with the fix.go build ./...,go vet, and the fullprograms/token+programs/token-2022suites are green; gofmt clean.Closes #460.
Same account-flag correctness class as #452 / #458 (folded into #453). Kept as a separate PR since it is a different program and does not overlap those diffs, but happy to fold it into #453 instead if you prefer a single account-flags PR.