diff --git a/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts b/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts index 90e2733291..43a7a59ef6 100644 --- a/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts +++ b/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts @@ -272,11 +272,17 @@ export function explainSolTransaction(params: ExplainTransactionWasmOptions): So } // --- Staking authorize --- + // A standard authorize tx contains two instructions: one for Staker and one + // for Withdrawer authority. Prefer the Withdrawer instruction for the + // stakingAuthorize summary because newWithdrawAddress is the security-critical + // field validated in verifyTransaction. Fall back to the first instruction + // if no Withdrawer instruction is present. let stakingAuthorize: StakingAuthorizeParams | undefined; for (const instr of parsed.instructionsData) { if (instr.type === 'StakingAuthorize') { - stakingAuthorize = mapStakingAuthorize(instr); - break; + if (!stakingAuthorize || instr.authorizeType === 'Withdrawer') { + stakingAuthorize = mapStakingAuthorize(instr); + } } } diff --git a/modules/sdk-coin-sol/src/lib/transaction.ts b/modules/sdk-coin-sol/src/lib/transaction.ts index e91bfcb1d3..83df2b5fdb 100644 --- a/modules/sdk-coin-sol/src/lib/transaction.ts +++ b/modules/sdk-coin-sol/src/lib/transaction.ts @@ -33,6 +33,7 @@ import { Memo, Nonce, StakingActivate, + StakingAuthorize, StakingAuthorizeParams, StakingWithdraw, TokenTransfer, @@ -540,6 +541,7 @@ export class Transaction extends BaseTransaction { const outputs: TransactionRecipient[] = []; // Create a separate array for token enablements const tokenEnablements: ITokenEnablement[] = []; + let stakingAuthorize: StakingAuthorizeParams | undefined = undefined; for (const instruction of decodedInstructions) { switch (instruction.type) { @@ -598,6 +600,36 @@ export class Transaction extends BaseTransaction { tokenAddress: ataInit.params.mintAddress, }); break; + case InstructionBuilderTypes.StakingAuthorize: { + const authorizeInstruction = instruction as StakingAuthorize; + // Neither instruction parser surfaces Solana's stakeAuthorizationType, so a + // Withdrawer-type authorize is identified by its custodian key: the standard + // parser surfaces it as newWithdrawAddress, the raw parser as custodianAddress. + // A standard authorize tx carries both a Staker and a Withdrawer instruction; + // the Withdrawer one wins because newWithdrawAddress is what verifyTransaction + // validates. Staker-only instructions must not populate the withdraw fields, + // otherwise a staker address would be compared against an intended withdraw key. + const isWithdrawerAuthorize = !!( + authorizeInstruction.params.newWithdrawAddress || authorizeInstruction.params.custodianAddress + ); + if (isWithdrawerAuthorize) { + stakingAuthorize = { + stakingAddress: authorizeInstruction.params.stakingAddress, + oldWithdrawAddress: authorizeInstruction.params.oldAuthorizeAddress, + newWithdrawAddress: authorizeInstruction.params.newAuthorizeAddress, + custodianAddress: authorizeInstruction.params.custodianAddress, + }; + } else if (!stakingAuthorize) { + stakingAuthorize = { + stakingAddress: authorizeInstruction.params.stakingAddress, + oldWithdrawAddress: '', + newWithdrawAddress: '', + oldStakingAuthorityAddress: authorizeInstruction.params.oldAuthorizeAddress, + newStakingAuthorityAddress: authorizeInstruction.params.newAuthorizeAddress, + }; + } + break; + } case InstructionBuilderTypes.CustomInstruction: // Custom instructions are arbitrary and cannot be explained break; @@ -617,7 +649,7 @@ export class Transaction extends BaseTransaction { } } - return this.getExplainedTransaction(outputAmount, outputs, memo, durableNonce, tokenEnablements); + return this.getExplainedTransaction(outputAmount, outputs, memo, durableNonce, tokenEnablements, stakingAuthorize); } private calculateFee(): string { @@ -638,7 +670,8 @@ export class Transaction extends BaseTransaction { outputs: TransactionRecipient[], memo: undefined | string = undefined, durableNonce: undefined | DurableNonceParams = undefined, - tokenEnablements: ITokenEnablement[] = [] + tokenEnablements: ITokenEnablement[] = [], + stakingAuthorize: StakingAuthorizeParams | undefined = undefined ): TransactionExplanation { const feeString = this.calculateFee(); @@ -674,6 +707,7 @@ export class Transaction extends BaseTransaction { blockhash: this.getNonce(), durableNonce: durableNonce, tokenEnablements: tokenEnablements, + ...(stakingAuthorize && { stakingAuthorize }), }; return explanation; diff --git a/modules/sdk-coin-sol/src/sol.ts b/modules/sdk-coin-sol/src/sol.ts index c426ebe05c..14a0b04fec 100644 --- a/modules/sdk-coin-sol/src/sol.ts +++ b/modules/sdk-coin-sol/src/sol.ts @@ -571,6 +571,48 @@ export class Sol extends BaseCoin { } } + const isStakingAuthorizeTx = + transaction.type === TransactionType.StakingAuthorize || + transaction.type === TransactionType.StakingAuthorizeRaw || + txParams.type === 'authorize'; + if (isStakingAuthorizeTx) { + const authorizeParams = explainedTx.stakingAuthorize; + if (!authorizeParams) { + throw new Error('StakingAuthorize transaction is missing stakingAuthorize explanation fields'); + } + // oldWithdrawAddress is '' for staker-only instructions (no Withdrawer authority change). + // Only validate when it is a non-empty string — an empty string indicates the instruction + // changes staker authority only, not withdrawer, so the wallet root check does not apply. + if ( + walletRootAddress && + authorizeParams.oldWithdrawAddress && + authorizeParams.oldWithdrawAddress !== walletRootAddress + ) { + throw new Error( + 'StakingAuthorize oldWithdrawAddress does not match wallet root address: expected ' + + walletRootAddress + + ' but got ' + + authorizeParams.oldWithdrawAddress + ); + } + if (txParams.newWithdrawPublicKey && authorizeParams.newWithdrawAddress !== txParams.newWithdrawPublicKey) { + throw new Error( + 'StakingAuthorize newWithdrawAddress does not match intended newWithdrawPublicKey: expected ' + + txParams.newWithdrawPublicKey + + ' but got ' + + authorizeParams.newWithdrawAddress + ); + } + if (txParams.stakeAccount && authorizeParams.stakingAddress !== txParams.stakeAccount) { + throw new Error( + 'StakingAuthorize stakingAddress does not match intended stakeAccount: expected ' + + txParams.stakeAccount + + ' but got ' + + authorizeParams.stakingAddress + ); + } + } + const isTokenEnablementTx = txParams.type === 'enabletoken'; // users do not input recipients for consolidation requests as they are generated by the server // Close-ATA txs do not populate explainedTx.outputs; recipients carry ATA addresses for intent only. diff --git a/modules/sdk-coin-sol/test/unit/sol.ts b/modules/sdk-coin-sol/test/unit/sol.ts index 9d8b4e4196..f15c95e7f0 100644 --- a/modules/sdk-coin-sol/test/unit/sol.ts +++ b/modules/sdk-coin-sol/test/unit/sol.ts @@ -1000,6 +1000,143 @@ describe('SOL:', function () { } as any); validTransaction.should.equal(true); }); + + describe('staking authorize transaction verification', function () { + const newWithdrawKey = new KeyPair(resources.authAccount2).getKeys(); + // a key that is not involved in the authorize tx — used to simulate malicious substitution + const differentKey = new KeyPair(resources.splitStakeAccount).getKeys(); + + const buildAuthorizeTx = async (newAuthorizedAddress: string) => { + const tx = await factory + .getStakingAuthorizeBuilder() + .stakingAddress(stakeAccount.pub) + .sender(wallet.pub) + .nonce(blockHash) + .newAuthorizedAddress(newAuthorizedAddress) + .oldAuthorizedAddress(wallet.pub) + .fee({ amount: 5000 }) + .build(); + return tx.toBroadcastFormat(); + }; + + it('should verify a valid staking authorize transaction with all intent fields', async function () { + const txBase64 = await buildAuthorizeTx(newWithdrawKey.pub); + const txParams = newTxParams(); + const txPrebuild = newTxPrebuild(); + txPrebuild.txBase64 = txBase64; + txPrebuild.txInfo.nonce = blockHash; + txParams.recipients = []; + txParams.type = 'authorize'; + txParams.newWithdrawPublicKey = newWithdrawKey.pub; + txParams.stakeAccount = stakeAccount.pub; + const result = await basecoin.verifyTransaction({ + txParams, + txPrebuild, + wallet: walletObj, + } as any); + result.should.equal(true); + }); + + it('should verify a valid staking authorize transaction without optional intent fields', async function () { + const txBase64 = await buildAuthorizeTx(newWithdrawKey.pub); + const txParams = newTxParams(); + const txPrebuild = newTxPrebuild(); + txPrebuild.txBase64 = txBase64; + txPrebuild.txInfo.nonce = blockHash; + txParams.recipients = []; + txParams.type = 'authorize'; + // newWithdrawPublicKey and stakeAccount not set — skips those checks + const result = await basecoin.verifyTransaction({ + txParams, + txPrebuild, + wallet: walletObj, + } as any); + result.should.equal(true); + }); + + it('should reject a staking authorize transaction where newWithdrawAddress was swapped to attacker key', async function () { + const txBase64 = await buildAuthorizeTx(differentKey.pub); + const txParams = newTxParams(); + const txPrebuild = newTxPrebuild(); + txPrebuild.txBase64 = txBase64; + txPrebuild.txInfo.nonce = blockHash; + txParams.recipients = []; + txParams.type = 'authorize'; + // Intent says newWithdrawPublicKey should be newWithdrawKey, but tx has attacker key + txParams.newWithdrawPublicKey = newWithdrawKey.pub; + txParams.stakeAccount = stakeAccount.pub; + await basecoin + .verifyTransaction({ txParams, txPrebuild, wallet: walletObj } as any) + .should.rejectedWith(/StakingAuthorize newWithdrawAddress does not match intended newWithdrawPublicKey/); + }); + + it('should reject a staking authorize transaction where stakeAccount does not match', async function () { + const txBase64 = await buildAuthorizeTx(newWithdrawKey.pub); + const txParams = newTxParams(); + const txPrebuild = newTxPrebuild(); + txPrebuild.txBase64 = txBase64; + txPrebuild.txInfo.nonce = blockHash; + txParams.recipients = []; + txParams.type = 'authorize'; + txParams.newWithdrawPublicKey = newWithdrawKey.pub; + // Pass a different stakeAccount (attacker has replaced it) + txParams.stakeAccount = differentKey.pub; + await basecoin + .verifyTransaction({ txParams, txPrebuild, wallet: walletObj } as any) + .should.rejectedWith(/StakingAuthorize stakingAddress does not match intended stakeAccount/); + }); + + it('should reject a staking authorize transaction where oldWithdrawAddress does not match wallet root', async function () { + // Build tx where oldAuthorizedAddress is NOT wallet.pub + const tx = await factory + .getStakingAuthorizeBuilder() + .stakingAddress(stakeAccount.pub) + .sender(newWithdrawKey.pub) + .nonce(blockHash) + .newAuthorizedAddress(newWithdrawKey.pub) + .oldAuthorizedAddress(newWithdrawKey.pub) // different from walletObj root + .fee({ amount: 5000 }) + .build(); + const txBase64 = tx.toBroadcastFormat(); + const txParams = newTxParams(); + const txPrebuild = newTxPrebuild(); + txPrebuild.txBase64 = txBase64; + txPrebuild.txInfo.nonce = blockHash; + txParams.recipients = []; + txParams.type = 'authorize'; + txParams.newWithdrawPublicKey = newWithdrawKey.pub; + txParams.stakeAccount = stakeAccount.pub; + await basecoin + .verifyTransaction({ txParams, txPrebuild, wallet: walletObj } as any) + .should.rejectedWith(/StakingAuthorize oldWithdrawAddress does not match wallet root address/); + }); + + it('should still enforce the fee payer check on a staking authorize transaction', async function () { + // oldAuthorizedAddress stays the wallet root so the authorize checks pass, but the + // fee payer is someone else — the authorize branch must not short-circuit that check. + const tx = await factory + .getStakingAuthorizeBuilder() + .stakingAddress(stakeAccount.pub) + .sender(wallet.pub) + .nonce(blockHash) + .newAuthorizedAddress(newWithdrawKey.pub) + .oldAuthorizedAddress(wallet.pub) + .feePayer(differentKey.pub) + .fee({ amount: 5000 }) + .build(); + const txParams = newTxParams(); + const txPrebuild = newTxPrebuild(); + txPrebuild.txBase64 = tx.toBroadcastFormat(); + txPrebuild.txInfo.nonce = blockHash; + txParams.recipients = []; + txParams.type = 'authorize'; + txParams.newWithdrawPublicKey = newWithdrawKey.pub; + txParams.stakeAccount = stakeAccount.pub; + await basecoin + .verifyTransaction({ txParams, txPrebuild, wallet: walletObj } as any) + .should.rejectedWith('Tx fee payer is not the wallet root address'); + }); + }); }); describe('getAmountBasedOnEndianness', () => { diff --git a/modules/sdk-coin-sol/test/unit/transaction.ts b/modules/sdk-coin-sol/test/unit/transaction.ts index a5f72c69d0..7fa561d871 100644 --- a/modules/sdk-coin-sol/test/unit/transaction.ts +++ b/modules/sdk-coin-sol/test/unit/transaction.ts @@ -1122,4 +1122,69 @@ describe('Sol Transaction', () => { }); }); }); + + describe('StakingAuthorize explainTransaction (non-WASM path)', () => { + // The 'sol' coin (mainnet) uses the legacy non-WASM explainTransaction path, + // unlike 'tsol' which routes through the WASM explainer. This ensures the new + // case InstructionBuilderTypes.StakingAuthorize block in transaction.ts is covered. + const solCoin = coins.get('sol'); + const factory = getBuilderFactory('sol'); + const wallet = new KeyPair(testData.authAccount).getKeys(); + const stakeAccount = new KeyPair(testData.stakeAccount).getKeys(); + const newWithdrawKey = new KeyPair(testData.authAccount2).getKeys(); + const blockHash = testData.blockHashes.validBlockHashes[0]; + + it('should populate stakingAuthorize with Withdrawer fields from a standard two-instruction authorize tx', async () => { + const tx = await factory + .getStakingAuthorizeBuilder() + .stakingAddress(stakeAccount.pub) + .sender(wallet.pub) + .nonce(blockHash) + .newAuthorizedAddress(newWithdrawKey.pub) + .oldAuthorizedAddress(wallet.pub) + .fee({ amount: 5000 }) + .build(); + + const rawTxBase64 = tx.toBroadcastFormat(); + const solTx = new Transaction(solCoin); + solTx.fromRawTransaction(rawTxBase64); + const explained = solTx.explainTransaction(); + + should.exist(explained.stakingAuthorize); + explained.stakingAuthorize!.stakingAddress.should.equal(stakeAccount.pub); + explained.stakingAuthorize!.oldWithdrawAddress.should.equal(wallet.pub); + explained.stakingAuthorize!.newWithdrawAddress.should.equal(newWithdrawKey.pub); + }); + + it('should leave withdraw fields empty for a staker-only authorize tx', async () => { + const tx = await factory + .getStakingAuthorizeBuilder() + .stakingAddress(stakeAccount.pub) + .sender(wallet.pub) + .nonce(blockHash) + .newAuthorizedAddress(newWithdrawKey.pub) + .oldAuthorizedAddress(wallet.pub) + .fee({ amount: 5000 }) + .build(); + + // Drop the Withdrawer instruction, keeping only the Staker authorize instruction. + const full = SolTransaction.from(Buffer.from(tx.toBroadcastFormat(), 'base64')); + const stakerOnly = new SolTransaction(); + stakerOnly.recentBlockhash = full.recentBlockhash; + stakerOnly.feePayer = full.feePayer; + stakerOnly.add(full.instructions[0]); + + const solTx = new Transaction(solCoin); + solTx.fromRawTransaction( + stakerOnly.serialize({ requireAllSignatures: false, verifySignatures: false }).toString('base64') + ); + const explained = solTx.explainTransaction(); + + should.exist(explained.stakingAuthorize); + explained.stakingAuthorize!.oldWithdrawAddress.should.equal(''); + explained.stakingAuthorize!.newWithdrawAddress.should.equal(''); + explained.stakingAuthorize!.oldStakingAuthorityAddress!.should.equal(wallet.pub); + explained.stakingAuthorize!.newStakingAuthorityAddress!.should.equal(newWithdrawKey.pub); + }); + }); }); diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index ccd5a4adf4..5a38227edc 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -287,6 +287,10 @@ export interface TransactionParams { memo?: Memo; enableTokens?: TokenEnablement[]; stakingRequestId?: string; + /** SOL authorize: new withdraw authority public key from the intent. */ + newWithdrawPublicKey?: string; + /** SOL authorize: stake account address from the intent. */ + stakeAccount?: string; } export interface AddressVerificationData { diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts index 96c4d14d88..98c1b63517 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -491,6 +491,10 @@ export interface PopulatedIntent extends PopulatedIntentBase, DefiIntentFields { clientOnboarder?: string; /** Optional ISO 8601 expiration timestamp (cantonParticipantOnboardingRequest intent). */ expirationIso?: string; + /** SOL authorize intent: new withdraw authority public key. */ + newWithdrawPublicKey?: string; + /** SOL authorize intent: stake account address being re-authorized. */ + stakeAccount?: string; } export type TxRequestState = diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts index d7116dea0d..994b3a714c 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -138,6 +138,17 @@ export function resolveEffectiveTxParams( effectiveTxParams.stakingRequestId = intentStakingRequestId; } + // Propagate SOL authorize-specific fields from the intent so sol.ts:verifyTransaction + // can validate the decoded instruction against what the user intended. + const intentNewWithdrawPublicKey = (txRequest.intent as PopulatedIntent)?.newWithdrawPublicKey; + if (intentNewWithdrawPublicKey && !effectiveTxParams.newWithdrawPublicKey) { + effectiveTxParams.newWithdrawPublicKey = intentNewWithdrawPublicKey; + } + const intentStakeAccount = (txRequest.intent as PopulatedIntent)?.stakeAccount; + if (intentStakeAccount && !effectiveTxParams.stakeAccount) { + effectiveTxParams.stakeAccount = intentStakeAccount; + } + // All staking intents (BSC delegate/undelegate, CELO stake/unstake, etc.) carry // stakingRequestId as a required field on BaseStakeIntent (@bitgo/public-types). // Use its presence as a generic staking signal — no need to enumerate every intentType. diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts index 72c452681f..829bdac2d6 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -371,6 +371,38 @@ describe('recipientUtils', function () { const txRequest = makeTxRequest({ intent: { intentType: 'stakingAuthorize' } as any }); assert.throws(() => resolveEffectiveTxParams(txRequest, {}), InvalidTransactionError); }); + + it('propagates newWithdrawPublicKey from authorize intent into effectiveTxParams', function () { + const txRequest = makeTxRequest({ + intent: { intentType: 'authorize', newWithdrawPublicKey: 'SomePubkey123' } as any, + }); + const result = resolveEffectiveTxParams(txRequest, {}); + assert.strictEqual(result.newWithdrawPublicKey, 'SomePubkey123'); + }); + + it('propagates stakeAccount from authorize intent into effectiveTxParams', function () { + const txRequest = makeTxRequest({ + intent: { intentType: 'authorize', stakeAccount: 'StakeAcct456' } as any, + }); + const result = resolveEffectiveTxParams(txRequest, {}); + assert.strictEqual(result.stakeAccount, 'StakeAcct456'); + }); + + it('does not overwrite existing newWithdrawPublicKey in txParams', function () { + const txRequest = makeTxRequest({ + intent: { intentType: 'authorize', newWithdrawPublicKey: 'IntentKey' } as any, + }); + const result = resolveEffectiveTxParams(txRequest, { newWithdrawPublicKey: 'CallerKey' } as any); + assert.strictEqual(result.newWithdrawPublicKey, 'CallerKey'); + }); + + it('does not overwrite existing stakeAccount in txParams', function () { + const txRequest = makeTxRequest({ + intent: { intentType: 'authorize', stakeAccount: 'IntentAcct' } as any, + }); + const result = resolveEffectiveTxParams(txRequest, { stakeAccount: 'CallerAcct' } as any); + assert.strictEqual(result.stakeAccount, 'CallerAcct'); + }); }); }); });