diff --git a/modules/sdk-coin-sol/src/lib/constants.ts b/modules/sdk-coin-sol/src/lib/constants.ts index 65a6d8d8cfa..cae85bb657d 100644 --- a/modules/sdk-coin-sol/src/lib/constants.ts +++ b/modules/sdk-coin-sol/src/lib/constants.ts @@ -268,6 +268,18 @@ export const marinadeStakingDeactivateInstructionsIndexes = { Memo: 1, } as const; +/** + * Marinade native-staking unstake has no on-chain program instruction: it is a single SystemProgram + * Transfer to one of Marinade's order addresses plus a PrepareForRevoke memo, acted on off-chain. + * The destination and amount are therefore the only server-independent signal that it is genuine. + */ +export const MARINADE_ORDER_ADDRESSES: readonly string[] = [ + 'opNS8ENpEMWdXcJUgJCsJTDp7arTXayoBEeBUg6UezP', // native + 'opiNSvKm1aPkbuSjnZfY5vyJUmaVxuQyTJt6jnNTW1L', // select +]; +/** Lamports BitGo sends per Marinade unstake order (marinade.unstakeTransferAmount in staking-service) */ +export const MARINADE_UNSTAKE_TRANSFER_LAMPORTS = BigInt(2300000); + /** Const to check the order of the Partial Staking Deactivate instructions when decoded */ export const stakingPartialDeactivateInstructionsIndexes = { Transfer: 0, diff --git a/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts b/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts index c2f92cfa4b5..481f4749f2b 100644 --- a/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts +++ b/modules/sdk-coin-sol/src/lib/explainTransactionWasm.ts @@ -9,6 +9,7 @@ import { } from './constants'; import { StakingAuthorizeParams, TransactionExplanation as SolLibTransactionExplanation } from './iface'; import { findTokenName } from './instructionParamsFactory'; +import { isMarinadeUnstakeTransfer, isPrepareForRevokeMemo } from './utils'; export interface ExplainTransactionWasmOptions { txBase64: string; @@ -129,15 +130,31 @@ function isWasmTokenAclThawInstruction(instr: InstructionParams): boolean { return instr.type === 'Unknown' && instr.programId === TOKEN_ACL_PROGRAM_ID; } -function deriveTransactionType( - instructions: InstructionParams[], - combined: CombinedPattern | null, - memo: string | undefined -): TransactionType { +/** + * True only for a genuine Marinade unstake: exactly one Transfer to a Marinade order address for the + * pinned amount, one well-formed PrepareForRevoke memo, and only nonce-advance / compute-budget besides. + * Mirrors getMarinadeUnstakeTransfer in utils.ts; the memo alone is attacker-controlled. + */ +function isWasmMarinadeUnstake(instructions: InstructionParams[]): boolean { + const transfers = instructions.filter((i) => i.type === 'Transfer'); + const memos = instructions.filter((i) => i.type === 'Memo'); + const others = instructions.filter((i) => !BOILERPLATE_TYPES.has(i.type) && i.type !== 'Transfer'); + if (transfers.length !== 1 || memos.length !== 1 || others.length > 0) return false; + const transfer = transfers[0]; + const memo = memos[0]; + return ( + transfer.type === 'Transfer' && + memo.type === 'Memo' && + isPrepareForRevokeMemo(memo.memo) && + isMarinadeUnstakeTransfer(transfer.toAddress, transfer.amount) + ); +} + +function deriveTransactionType(instructions: InstructionParams[], combined: CombinedPattern | null): TransactionType { if (combined) return TransactionType[combined.kind]; - // Marinade deactivate: Transfer + memo containing "PrepareForRevoke" - if (memo?.includes('PrepareForRevoke')) return TransactionType.StakingDeactivate; + // Marinade deactivate: validated by transfer destination/amount, never by the memo alone + if (isWasmMarinadeUnstake(instructions)) return TransactionType.StakingDeactivate; // Jito pool operations if (instructions.some((i) => i.type === 'StakePoolDepositSol')) return TransactionType.StakingActivate; @@ -255,12 +272,10 @@ export function explainSolTransaction(params: ExplainTransactionWasmOptions): So // --- Detect combined patterns and derive type --- const combined = detectCombinedPattern(parsed.instructionsData); - const txType = deriveTransactionType(parsed.instructionsData, combined, memo); + const txType = deriveTransactionType(parsed.instructionsData, combined); - // Marinade deactivate: Transfer + PrepareForRevoke memo. - // The Transfer is a contract interaction, not real value transfer — skip from outputs. - const isMarinadeDeactivate = - txType === TransactionType.StakingDeactivate && memo !== undefined && memo.includes('PrepareForRevoke'); + // Genuine Marinade deactivate: the Transfer is a contract interaction, not a real value transfer. + const isMarinadeDeactivate = isWasmMarinadeUnstake(parsed.instructionsData); // --- Extract outputs and inputs --- const outputs: { address: string; amount: bigint; tokenName?: string }[] = []; diff --git a/modules/sdk-coin-sol/src/lib/instructionParamsFactory.ts b/modules/sdk-coin-sol/src/lib/instructionParamsFactory.ts index ac0b4819712..9302f19ca88 100644 --- a/modules/sdk-coin-sol/src/lib/instructionParamsFactory.ts +++ b/modules/sdk-coin-sol/src/lib/instructionParamsFactory.ts @@ -788,6 +788,10 @@ function parseStakingDeactivateInstructions( ): Array { const instructionData: Array = []; const unstakingInstructions: UnstakingInstructions[] = []; + // Jito unstake legitimately carries Approve + Create alongside WithdrawStake + const isJitoUnstake = instructions.some( + (instruction) => getInstructionType(instruction) === ValidInstructionTypesEnum.WithdrawStake + ); for (const instruction of instructions) { const type = getInstructionType(instruction); switch (type) { @@ -896,6 +900,16 @@ function parseStakingDeactivateInstructions( }); } break; + + case ValidInstructionTypesEnum.Approve: + case ValidInstructionTypesEnum.Create: + if (!isJitoUnstake) { + throw new NotSupported('Invalid deactivate stake transaction, instruction type not supported: ' + type); + } + break; + + default: + throw new NotSupported('Invalid deactivate stake transaction, instruction type not supported: ' + type); } } @@ -1099,6 +1113,9 @@ function parseStakingWithdrawInstructions( }; instructionData.push(stakingWithdraw); break; + + default: + throw new NotSupported('Invalid withdraw stake transaction, instruction type not supported: ' + type); } } diff --git a/modules/sdk-coin-sol/src/lib/utils.ts b/modules/sdk-coin-sol/src/lib/utils.ts index 09e74e91f76..0a66bac1465 100644 --- a/modules/sdk-coin-sol/src/lib/utils.ts +++ b/modules/sdk-coin-sol/src/lib/utils.ts @@ -62,6 +62,8 @@ import { ZK_ELGAMAL_PROOF_PROGRAM_ID, CT_EXT_DISCRIMINATOR, CT_SUB_DISCRIMINATORS, + MARINADE_ORDER_ADDRESSES, + MARINADE_UNSTAKE_TRANSFER_LAMPORTS, } from './constants'; import { ValidInstructionTypes } from './iface'; import { STAKE_POOL_INSTRUCTION_LAYOUTS, STAKE_POOL_PROGRAM_ID } from '@solana/spl-stake-pool'; @@ -363,6 +365,57 @@ function isConfidentialTransferRawInstruction(instruction: TransactionInstructio return false; } +/** True if the memo is a well-formed Marinade unstake memo: {"PrepareForRevoke":{"user":..,"amount":..}} */ +export function isPrepareForRevokeMemo(memo: string | undefined): boolean { + if (!memo) return false; + try { + const body = JSON.parse(memo)?.PrepareForRevoke; + return ( + typeof body === 'object' && body !== null && typeof body.user === 'string' && /^\d+$/.test(body.amount ?? '') + ); + } catch { + return false; + } +} + +/** True if a SOL transfer matches the single transfer BitGo sends to Marinade's order address per unstake. */ +export function isMarinadeUnstakeTransfer(toAddress: string, lamports: bigint | number | string): boolean { + return MARINADE_ORDER_ADDRESSES.includes(toAddress) && BigInt(lamports) === MARINADE_UNSTAKE_TRANSFER_LAMPORTS; +} + +/** + * Returns the decoded transfer if the instructions are a genuine Marinade unstake, otherwise undefined. + * Genuine means exactly one SOL Transfer to a Marinade order address for the pinned amount, one well-formed + * PrepareForRevoke memo, and nothing else except nonce-advance / compute-budget. The memo alone is + * attacker-controlled, so it must never decide the type by itself. + */ +export function getMarinadeUnstakeTransfer( + instructions: TransactionInstruction[] +): ReturnType | undefined { + const transfers: TransactionInstruction[] = []; + const memos: TransactionInstruction[] = []; + for (const instruction of instructions) { + switch (getInstructionType(instruction)) { + case ValidInstructionTypesEnum.AdvanceNonceAccount: + case ValidInstructionTypesEnum.SetComputeUnitLimit: + case ValidInstructionTypesEnum.SetPriorityFee: + break; + case ValidInstructionTypesEnum.Memo: + memos.push(instruction); + break; + case ValidInstructionTypesEnum.Transfer: + transfers.push(instruction); + break; + default: + return undefined; + } + } + if (transfers.length !== 1 || memos.length !== 1) return undefined; + if (!isPrepareForRevokeMemo(memos[0].data.toString('utf-8'))) return undefined; + const transfer = SystemInstruction.decodeTransfer(transfers[0]); + return isMarinadeUnstakeTransfer(transfer.toPubkey.toString(), transfer.lamports) ? transfer : undefined; +} + /** * Returns the transaction Type based on the transaction instructions. * Wallet initialization, Transfer and Staking transactions are supported. @@ -395,11 +448,12 @@ export function getTransactionType(transaction: SolTransaction): TransactionType } if (instructions.filter((instruction) => getInstructionType(instruction) === 'Deactivate').length === 0) { + // A Transfer is a Send unless the whole transaction is a genuine Marinade unstake (see getMarinadeUnstakeTransfer) + const isMarinadeUnstake = getMarinadeUnstakeTransfer(instructions) !== undefined; for (const instruction of instructions) { const instructionType = getInstructionType(instruction); - // Check if memo instruction is there and if it contains 'PrepareForRevoke' because Marinade staking deactivate transaction will have this if ( - (instructionType === ValidInstructionTypesEnum.Transfer && !memoData?.includes('PrepareForRevoke')) || + (instructionType === ValidInstructionTypesEnum.Transfer && !isMarinadeUnstake) || instructionType === ValidInstructionTypesEnum.TokenTransfer ) { return TransactionType.Send; diff --git a/modules/sdk-coin-sol/src/sol.ts b/modules/sdk-coin-sol/src/sol.ts index 09a5fff3b74..8f2150566be 100644 --- a/modules/sdk-coin-sol/src/sol.ts +++ b/modules/sdk-coin-sol/src/sol.ts @@ -95,6 +95,7 @@ import { } from './lib/token2022Resolve'; import { getAssociatedTokenAccountAddress, + getMarinadeUnstakeTransfer, getSolTokenFromAddress, getSolTokenFromTokenName, isValidAddress, @@ -610,6 +611,15 @@ export class Sol extends BaseCoin { const explainedTx = transaction.explainTransaction(); const isCloseAssociatedTokenAccountTx = transaction.type === TransactionType.CloseAssociatedTokenAccount; + // A Marinade unstake produces no explained outputs, so the only thing tying its transfer to this wallet + // is the source. The source is the wallet root, not the fee payer, since sponsored fee payers exist. + if (transaction.type === TransactionType.StakingDeactivate) { + const marinadeTransfer = getMarinadeUnstakeTransfer(transaction.solTransaction.instructions); + if (marinadeTransfer && marinadeTransfer.fromPubkey.toString() !== walletRootAddress) { + throw new Error('Marinade unstake transfer must be sent from the wallet root address'); + } + } + if (txParams.type === 'enabletoken' && verificationOptions?.verifyTokenEnablement) { this.verifyTxType(txParams.type, explainedTx.type); const tokenEnablementsPrebuild = this.throwIfMissingTokenEnablementsOrReturn(explainedTx); diff --git a/modules/sdk-coin-sol/test/resources/sol.ts b/modules/sdk-coin-sol/test/resources/sol.ts index 527b6ff7a7e..d31cdb13961 100644 --- a/modules/sdk-coin-sol/test/resources/sol.ts +++ b/modules/sdk-coin-sol/test/resources/sol.ts @@ -205,7 +205,7 @@ export const STAKING_DEACTIVATE_SIGNED_TX = 'AUfyWtl4IUxhH21qX/H03hJZer1XxQaxL2r/uDTM/u1GzBIyePCHu78O2SkWGEYP6eDdiY3OLfJmUM1jiy8NCAoBAAIEReV5vPklPPaLR9/x+zo6XCwhusWyPAmuEqbgVWvwi0Fiey+6ASdh+bkZvPlMu0ydyAUdnwkymTFNOUkjMmi96Qah2BeRN1QqmDQ3vf4qerJVf1NcinhyK2ikncAAAAAABqfVFxjHdMkoVmOYaR1etoteuKObS21cc1VbIQAAAADjMtr5L6vs6LY/96RABeX9/Zr6FYdWthxalfkEs7jQgQECAwEDAAQFAAAA'; export const MARINADE_STAKING_DEACTIVATE_SIGNED_TX = - 'AaiHUOzzeJaUzpdkm2BmLJI3AVOhFHTD5BnVMwUH3lRv8hKpH1fnXiXNm6ghZgNwhggXBAqhL3t4XEl+H7T95gIBAAIEReV5vPklPPaLR9/x+zo6XCwhusWyPAmuEqbgVWvwi0EL/kczKamI94jNtLT/BR9nkfa/PR2IU6d7qaEV8VVC3AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABUpTWpkpIQZNJOhxYNo4fHw1td28kruB5B+oQEEFRI3jMtr5L6vs6LY/96RABeX9/Zr6FYdWthxalfkEs7jQgQICAgABDAIAAABgGCMAAAAAAAMAbntcIlByZXBhcmVGb3JSZXZva2VcIjp7XCJ1c2VyXCI6XCI1aHI1ZmlzUGk2RFhOdXVScG01WFVienBpRW5tZHl4WHVCRFR3endaajVQZX1cIixcImFtb3VudFwiOlwiNTAwMDAwMDAwMDAwXCJ9'; + 'AWmhrcrX9qzw++JsZqmjFwPT41NpnDqbSUe5P34yCMBYsJcNKgV6o1KWgZZc9qfimToM/uf/c5ihnnr+6adZiwcBAAIEReV5vPklPPaLR9/x+zo6XCwhusWyPAmuEqbgVWvwi0EL/kczKamI94jNtLT/BR9nkfa/PR2IU6d7qaEV8VVC3AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABUpTWpkpIQZNJOhxYNo4fHw1td28kruB5B+oQEEFRI3jMtr5L6vs6LY/96RABeX9/Zr6FYdWthxalfkEs7jQgQICAgABDAIAAABgGCMAAAAAAAMAZHsiUHJlcGFyZUZvclJldm9rZSI6eyJ1c2VyIjoiNWhyNWZpc1BpNkRYTnV1UnBtNVhVYnpwaUVubWR5eFh1QkRUd3p3Wmo1UGUiLCJhbW91bnQiOiI1MDAwMDAwMDAwMDAifX0='; export const JITO_STAKING_DEACTIVATE_SIGNED_TX = 'A7txZr55CtSJogfV1ihB1JOIuVbmhAh7BCl4hJeTBcbrq6KT+Jzbbjx4qEXDgRnMtY7cb9xnekOHUfKkW9D2RQpchl2oH4Np/+Oghy7QNjKrodZsFlqhiYoo+Zx0Bjf+Hwq35h/zVd1kHRTkaB1ebZwDeEejPrFgNCpkqxRh9ZgOMBethjkNPCrqzk50pOqx1ktJik5loScyp/81bggjQASE4jMdtET/a2jpFJeG34GZLIY6r+LNTXtGsK53qyR9CQMBBg9F5Xm8+SU89otH3/H7OjpcLCG6xbI8Ca4SpuBVa/CLQWJ7L7oBJ2H5uRm8+Uy7TJ3IBR2fCTKZMU05SSMyaL3p6Lfi7T4mzoclKbPedsv+JDs60KtRcBK6Y7CHyYejKikcg6pe+BOG2OETfAVS9ftz6va1oE4onLBolJ2N+ZOOhCPgdQm63e39tRapC5GXu1BHQyVdDjfF/13OiiQe7cQxl9rD5vZLBx1Aaz7SV4hmv2ZhGp4LQEU67b++EtDrIi8J5qP+7PmQMuHB32uXItyzY057jjRAk2vDSwzByOtSH/zRQemDLK8QrZF0lcoPJxtbKTzUcCfqc3AH7UDrOaC9BIo+CMO0lb4X9FQn2JvsW4DH4mlcGGTXZ0PbOb7TRtYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFTlniTAUaihSnsel5fw4Szfp0kCFmUxlaalEqbxZmArBoFO1Mr2ihdGcv2shgMaY+hOoV76HUS3IpP229sAFlAGodgXkTdUKpg0N73+KnqyVX9TXIp4citopJ3AAAAAAAan1RcYx3TJKFZjmGkdXraLXrijm0ttXHNVWyEAAAAABt324ddloZPZy+FGzut5rBy0he1fWzeROoz1hX7/AKnjMtr5L6vs6LY/96RABeX9/Zr6FYdWthxalfkEs7jQgQQOAwMCAAkE6AMAAAAAAAAJAgABNAAAAACA1SIAAAAAAMgAAAAAAAAABqHYF5E3VCqYNDe9/ip6slV/U1yKeHIraKSdwAAAAAALDQgECgUBAAIDBgcNDgwJCugDAAAAAAAADAMBDQAEBQAAAA=='; diff --git a/modules/sdk-coin-sol/test/unit/explainTransactionWasm.ts b/modules/sdk-coin-sol/test/unit/explainTransactionWasm.ts index 24e46f5e2d5..6629e96463d 100644 --- a/modules/sdk-coin-sol/test/unit/explainTransactionWasm.ts +++ b/modules/sdk-coin-sol/test/unit/explainTransactionWasm.ts @@ -3,8 +3,63 @@ */ import 'should'; import { explainSolTransaction } from '../../src/lib/explainTransactionWasm'; +import { TransactionType } from '@bitgo/sdk-core'; +import { KeyPair } from '../../src'; +import * as testData from '../resources/sol'; +import { getBuilderFactory } from './getBuilderFactory'; describe('explainTransactionWasm', function () { + describe('PrepareForRevoke memo', function () { + const factory = getBuilderFactory('tsol'); + const sender = new KeyPair(testData.authAccount).getKeys().pub; + const attacker = 'CP5Dpaa42RtJmMuKqCQsLwma5Yh3knuvKsYDFX85F41S'; + const marinadeOrder = 'opNS8ENpEMWdXcJUgJCsJTDp7arTXayoBEeBUg6UezP'; + const blockhash = 'GHtXQBsoZHVnNFa9YevAzFr17DJjgHXk3ycTKD5xD3Zi'; + const validMemo = `{"PrepareForRevoke":{"user":"${sender}","amount":"500000000000"}}`; + + const buildTransfer = async (to: string, amount: string, memo: string) => { + const builder = factory.getTransferBuilder(); + builder.sender(sender).nonce(blockhash).send({ address: to, amount }); + builder.memo(memo); + return (await builder.build()).toBroadcastFormat(); + }; + + // The legacy path derives the type when parsing a raw tx, so rebuild it from raw like verifyTransaction does. + const explainBoth = async (rawTx: string) => { + const wasm = explainSolTransaction({ txBase64: rawTx, feeInfo: { fee: '5000' }, coinName: 'tsol' }); + const legacyTx = await factory.from(rawTx).build(); + return { wasm, legacy: legacyTx.explainTransaction(), legacyType: legacyTx.type }; + }; + + it('should classify a memo-tagged transfer to an arbitrary address as Send and keep its output', async function () { + const { wasm, legacy, legacyType } = await explainBoth(await buildTransfer(attacker, '1000', validMemo)); + wasm.type.should.equal('Send'); + wasm.outputs.should.deepEqual([{ address: attacker, amount: '1000' }]); + legacyType.should.equal(TransactionType.Send); + legacy.outputs.should.deepEqual([{ address: attacker, amount: '1000' }]); + }); + + it('should classify a Marinade-address transfer with the wrong amount as Send', async function () { + const { wasm, legacyType } = await explainBoth(await buildTransfer(marinadeOrder, '3000000', validMemo)); + wasm.type.should.equal('Send'); + legacyType.should.equal(TransactionType.Send); + }); + + it('should classify a Marinade-address transfer with a malformed PrepareForRevoke memo as Send', async function () { + const { wasm, legacyType } = await explainBoth(await buildTransfer(marinadeOrder, '2300000', 'PrepareForRevoke')); + wasm.type.should.equal('Send'); + legacyType.should.equal(TransactionType.Send); + }); + + it('should still classify a genuine Marinade unstake as StakingDeactivate with no outputs', async function () { + const { wasm, legacy, legacyType } = await explainBoth(testData.MARINADE_STAKING_DEACTIVATE_SIGNED_TX); + wasm.type.should.equal('StakingDeactivate'); + wasm.outputs.length.should.equal(0); + legacyType.should.equal(TransactionType.StakingDeactivate); + legacy.outputs.length.should.equal(0); + }); + }); + describe('deriveTransactionType', function () { it('should classify boilerplate-only transaction as CustomTx', function () { // Transaction with only NonceAdvance + Memo instructions (no Transfer or TokenTransfer). diff --git a/modules/sdk-coin-sol/test/unit/instructionParamsFactory.staking.ts b/modules/sdk-coin-sol/test/unit/instructionParamsFactory.staking.ts index 49b8cc45b32..9d7e2c329e8 100644 --- a/modules/sdk-coin-sol/test/unit/instructionParamsFactory.staking.ts +++ b/modules/sdk-coin-sol/test/unit/instructionParamsFactory.staking.ts @@ -417,7 +417,7 @@ describe('Instruction Parser Staking Tests: ', function () { should.deepEqual(result, instructionsData); }); - it('Should parse deactivate stake tx instructions if there are unexpected instructions', () => { + it('Should reject deactivate stake tx instructions if there are unexpected instructions', () => { const fromAccount = new PublicKey(testData.authAccount.pub); const stakingAccount = new PublicKey(testData.stakeAccount.pub); const validator = new PublicKey(testData.validator.pub); @@ -446,26 +446,14 @@ describe('Instruction Parser Staking Tests: ', function () { stakePubkey: stakingAccount, }).instructions; - // Params - const stakingActivateParams: StakingDeactivate = { - type: InstructionBuilderTypes.StakingDeactivate, - params: { - fromAddress: fromAccount.toString(), - stakingAddress: stakingAccount.toString(), - amount: undefined, - unstakingAddress: undefined, - stakingType: SolStakingTypeEnum.NATIVE, - }, - }; - const instructions = [ ...stakingActivateInstructions, ...stakingDelegateInstructions, ...stakingDeactivateInstructions, ]; - const instructionsData = [stakingActivateParams]; - const result = instructionParamsFactory(TransactionType.StakingDeactivate, instructions); - should.deepEqual(result, instructionsData); + should(() => instructionParamsFactory(TransactionType.StakingDeactivate, instructions)).throw( + 'Invalid deactivate stake transaction, instruction type not supported: Create' + ); }); describe('Partially deactivate stake instructions', function () { @@ -512,7 +500,7 @@ describe('Instruction Parser Staking Tests: ', function () { const instructions = [ nonceAdvanceInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -563,7 +551,7 @@ describe('Instruction Parser Staking Tests: ', function () { const instructions = [ nonceAdvanceInstruction, allocateInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -664,7 +652,7 @@ describe('Instruction Parser Staking Tests: ', function () { nonceAdvanceInstruction, allocateInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -721,7 +709,7 @@ describe('Instruction Parser Staking Tests: ', function () { nonceAdvanceInstruction, allocateInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -779,7 +767,7 @@ describe('Instruction Parser Staking Tests: ', function () { nonceAdvanceInstruction, allocateInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -837,7 +825,7 @@ describe('Instruction Parser Staking Tests: ', function () { nonceAdvanceInstruction, allocateInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -895,7 +883,7 @@ describe('Instruction Parser Staking Tests: ', function () { nonceAdvanceInstruction, allocateInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -952,7 +940,7 @@ describe('Instruction Parser Staking Tests: ', function () { nonceAdvanceInstruction, allocateInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -1040,7 +1028,7 @@ describe('Instruction Parser Staking Tests: ', function () { transferInstruction, allocateInstruction, assignInstruction, - ...splitInstructions, + splitInstructions[1], ...stakingDeactivateInstructions, memoInstruction, ]; @@ -1185,7 +1173,7 @@ describe('Instruction Parser Staking Tests: ', function () { should.deepEqual(result, instructionsData); }); - it('Should parse withdraw stake tx instructions if there are unexpected instructions', () => { + it('Should reject withdraw stake tx instructions if there are unexpected instructions', () => { const fromAccount = new PublicKey(testData.authAccount.pub); const stakingAccount = new PublicKey(testData.stakeAccount.pub); const validator = new PublicKey(testData.validator.pub); @@ -1216,24 +1204,14 @@ describe('Instruction Parser Staking Tests: ', function () { lamports: new BigNumber(amount).toNumber(), }).instructions; - // Params - const withdrawStakeParams: StakingWithdraw = { - type: InstructionBuilderTypes.StakingWithdraw, - params: { - fromAddress: fromAccount.toString(), - stakingAddress: stakingAccount.toString(), - amount, - }, - }; - const instructions = [ ...stakingActivateInstructions, ...stakingDelegateInstructions, ...withdrawStakeInstructions, ]; - const instructionsData = [withdrawStakeParams]; - const result = instructionParamsFactory(TransactionType.StakingWithdraw, instructions); - should.deepEqual(result, instructionsData); + should(() => instructionParamsFactory(TransactionType.StakingWithdraw, instructions)).throw( + 'Invalid withdraw stake transaction, instruction type not supported: Create' + ); }); }); }); diff --git a/modules/sdk-coin-sol/test/unit/sol.ts b/modules/sdk-coin-sol/test/unit/sol.ts index c2fa5128254..c306848714e 100644 --- a/modules/sdk-coin-sol/test/unit/sol.ts +++ b/modules/sdk-coin-sol/test/unit/sol.ts @@ -822,6 +822,74 @@ describe('SOL:', function () { .should.be.rejectedWith(/does not match the declared sponsored fee payer/); }); + describe('PrepareForRevoke memo', () => { + const attacker = 'CP5Dpaa42RtJmMuKqCQsLwma5Yh3knuvKsYDFX85F41S'; + const marinadeOrder = 'opNS8ENpEMWdXcJUgJCsJTDp7arTXayoBEeBUg6UezP'; + const memoFor = (user: string) => `{"PrepareForRevoke":{"user":"${user}","amount":"500000000000"}}`; + + const buildRaw = async (from: string, to: string, amount: string, memoText: string) => { + const txBuilder = factory.getTransferBuilder(); + txBuilder.sender(from); + txBuilder.nonce(blockHash); + txBuilder.send({ address: to, amount }); + txBuilder.memo(memoText); + return (await txBuilder.build()).toBroadcastFormat(); + }; + + it('should reject a consolidation prebuild that drains to an attacker behind the memo', async function () { + const txBase64 = await buildRaw(wallet.pub, attacker, '1000000000', memoFor(wallet.pub)); + await basecoin + .verifyTransaction({ + txParams: {}, + txPrebuild: { txBase64, consolidateId: 'consolidateId' }, + verification: { consolidationToBaseAddress: true }, + feePayer: wallet.pub, + blockhash: blockHash, + wallet: walletObj, + } as any) + .should.be.rejectedWith('tx outputs does not match with expected address'); + }); + + it('should reject a payment whose recipients do not match once the memo no longer hides the output', async function () { + const txBase64 = await buildRaw(wallet.pub, attacker, '1000', memoFor(wallet.pub)); + await basecoin + .verifyTransaction({ + txParams: { recipients: [] }, + txPrebuild: { txBase64 }, + feePayer: wallet.pub, + blockhash: blockHash, + wallet: walletObj, + } as any) + .should.be.rejectedWith('Number of tx outputs does not match with number of txParams recipients'); + }); + + it('should reject a Marinade unstake whose transfer is not sent from the wallet root', async function () { + const otherSender = stakeAccount.pub; + const txBase64 = await buildRaw(otherSender, marinadeOrder, '2300000', memoFor(otherSender)); + await basecoin + .verifyTransaction({ + txParams: {}, + txPrebuild: { txBase64 }, + feePayer: otherSender, + blockhash: blockHash, + wallet: walletObj, + } as any) + .should.be.rejectedWith('Marinade unstake transfer must be sent from the wallet root address'); + }); + + it('should accept a genuine Marinade unstake sent from the wallet root', async function () { + const txBase64 = await buildRaw(wallet.pub, marinadeOrder, '2300000', memoFor(wallet.pub)); + const result = await basecoin.verifyTransaction({ + txParams: {}, + txPrebuild: { txBase64 }, + feePayer: wallet.pub, + blockhash: blockHash, + wallet: walletObj, + } as any); + result.should.equal(true); + }); + }); + it('should verify a builder-produced transaction end-to-end with a sponsored fee payer', async function () { // Build a real transaction with the fee payer set to a different address, // serialize it, and verify it passes through verifyTransaction with the diff --git a/modules/sdk-coin-sol/test/unit/transactionBuilder/stakingDeactivateBuilder.ts b/modules/sdk-coin-sol/test/unit/transactionBuilder/stakingDeactivateBuilder.ts index d381fca2a19..ad45c33e3af 100644 --- a/modules/sdk-coin-sol/test/unit/transactionBuilder/stakingDeactivateBuilder.ts +++ b/modules/sdk-coin-sol/test/unit/transactionBuilder/stakingDeactivateBuilder.ts @@ -244,7 +244,7 @@ describe('Sol Staking Deactivate Builder', () => { amount: '2300000', }); - const marinadeMemo = `{\\"PrepareForRevoke\\":{\\"user\\":\\"${wallet.pub}}\\",\\"amount\\":\\"500000000000\\"}`; + const marinadeMemo = `{"PrepareForRevoke":{"user":"${wallet.pub}","amount":"500000000000"}}`; it('Marinade: build and sign a staking deactivate tx', async () => { await performTest({