feat: add explainSolTransaction using @bitgo/wasm-solana#7957
feat: add explainSolTransaction using @bitgo/wasm-solana#7957
Conversation
d32c089 to
bb885ad
Compare
f5732ce to
97c511d
Compare
OttoAllmendinger
left a comment
There was a problem hiding this comment.
we can move towards bigint a little bit more
where external interfaces force us to string keep it
| ]; | ||
|
|
||
| const outputs: { address: string; amount: string; memo?: string }[] = []; | ||
| let outputAmount = '0'; |
There was a problem hiding this comment.
| let outputAmount = '0'; | |
| let outputAmount = 0n; |
| address: instr.params.toAddress, | ||
| amount: instr.params.amount, | ||
| }); | ||
| outputAmount = (BigInt(outputAmount) + BigInt(instr.params.amount)).toString(); |
There was a problem hiding this comment.
let's avoid the internal bigint casts - why is instr.params.amount not a bigint already?
modules/sdk-coin-sol/src/sol.ts
Outdated
| // Derive outputs and tokenEnablements from combined instructions | ||
| const outputs: TransactionRecipient[] = []; | ||
| const tokenEnablements: ITokenEnablement[] = []; | ||
| let outputAmount = new BigNumber(0); |
There was a problem hiding this comment.
yet another way to sum up things... use bigint instead
|
|
||
| /** Transaction ID (first signature, base58 encoded) */ | ||
| get id(): string { | ||
| if (!this._wasmTransaction) return UNAVAILABLE_TEXT; |
There was a problem hiding this comment.
I'm guessing the interface forces us to do this ugliness?
There was a problem hiding this comment.
Unfortunately yes, base class forces the return type of string while _id can be string | undefined so it's a bit silly. We have to return a string, i guess we can return an empty string.
Seeing what it would take to update the base class to accept undefined as a return type. It would cause some breaking changes though but lets see. 🤖
There was a problem hiding this comment.
Fixing the interface could spiral out of control very quickly, so let's leave it. I just need to know what is hard to change and what is easy to change. You can use the override keyword in typescript to signal that a method or getter implements a base method.
38e4bc7 to
da3dc95
Compare
0b3225b to
3cb4054
Compare
OttoAllmendinger
left a comment
There was a problem hiding this comment.
counting on you claude
modules/sdk-coin-sol/src/sol.ts
Outdated
| let outputAmount = 0n; | ||
|
|
||
| for (const instr of combinedInstructions) { | ||
| switch (instr.type) { | ||
| case InstructionBuilderTypes.Transfer: | ||
| outputs.push({ | ||
| address: instr.params.toAddress, | ||
| amount: instr.params.amount, | ||
| }); | ||
| outputAmount += BigInt(instr.params.amount); | ||
| break; | ||
|
|
||
| case InstructionBuilderTypes.TokenTransfer: | ||
| outputs.push({ | ||
| address: instr.params.toAddress, | ||
| amount: instr.params.amount, | ||
| tokenName: findTokenName(instr.params.tokenAddress ?? '', undefined, true), | ||
| }); | ||
| break; | ||
|
|
||
| case InstructionBuilderTypes.CreateNonceAccount: | ||
| outputs.push({ | ||
| address: instr.params.nonceAddress, | ||
| amount: instr.params.amount, | ||
| }); | ||
| outputAmount += BigInt(instr.params.amount); | ||
| break; | ||
|
|
||
| case InstructionBuilderTypes.StakingActivate: | ||
| outputs.push({ | ||
| address: instr.params.stakingAddress, | ||
| amount: instr.params.amount, | ||
| }); | ||
| outputAmount += BigInt(instr.params.amount); | ||
| break; | ||
|
|
||
| case InstructionBuilderTypes.StakingWithdraw: | ||
| outputs.push({ | ||
| address: instr.params.fromAddress, | ||
| amount: instr.params.amount, | ||
| }); | ||
| outputAmount += BigInt(instr.params.amount); | ||
| break; | ||
|
|
||
| case InstructionBuilderTypes.CreateAssociatedTokenAccount: | ||
| tokenEnablements.push({ | ||
| address: instr.params.ataAddress, | ||
| tokenName: findTokenName(instr.params.mintAddress, undefined, true), | ||
| tokenAddress: instr.params.mintAddress, | ||
| }); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
this whole thing gives me a deja vue
@claude can we consolidate here
There was a problem hiding this comment.
🤖 Claude here.
Yes, this output extraction loop is duplicated in three places:
Transaction.explainTransaction()(transaction.ts:525-587) usingBigNumberWasmTransaction.explainTransaction()(wasm/transaction.ts:206-240) usingbigintSol.explainTransactionWithWasm()(sol.ts:810-860) usingbigint
Proposal: extract a shared free function in a new lib/explainUtil.ts:
interface ExplainedOutputs {
outputs: TransactionRecipient[];
tokenEnablements: ITokenEnablement[];
outputAmount: bigint;
memo?: string;
}
function extractOutputsFromInstructions(instructions: InstructionParams[]): ExplainedOutputs {
const outputs: TransactionRecipient[] = [];
const tokenEnablements: ITokenEnablement[] = [];
let outputAmount = 0n;
let memo: string | undefined;
for (const instr of instructions) {
switch (instr.type) {
case InstructionBuilderTypes.Transfer:
outputs.push({ address: instr.params.toAddress, amount: instr.params.amount });
outputAmount += BigInt(instr.params.amount);
break;
case InstructionBuilderTypes.TokenTransfer:
outputs.push({
address: instr.params.toAddress,
amount: instr.params.amount,
tokenName: instr.params.tokenName,
});
break;
case InstructionBuilderTypes.CreateNonceAccount:
outputs.push({ address: instr.params.nonceAddress, amount: instr.params.amount });
outputAmount += BigInt(instr.params.amount);
break;
case InstructionBuilderTypes.StakingActivate:
outputs.push({ address: instr.params.stakingAddress, amount: instr.params.amount });
outputAmount += BigInt(instr.params.amount);
break;
case InstructionBuilderTypes.StakingWithdraw:
outputs.push({ address: instr.params.fromAddress, amount: instr.params.amount });
outputAmount += BigInt(instr.params.amount);
break;
case InstructionBuilderTypes.CreateAssociatedTokenAccount:
tokenEnablements.push({
address: instr.params.ataAddress,
tokenName: instr.params.tokenName,
tokenAddress: instr.params.mintAddress,
});
break;
case InstructionBuilderTypes.Memo:
memo = instr.params.memo;
break;
}
}
return { outputs, tokenEnablements, outputAmount, memo };
}All three call sites then become thin wrappers that call this, eliminating the duplication. The legacy Transaction.explainTransaction() can convert the bigint result to BigNumber at the boundary if needed (or migrate to bigint too).
modules/sdk-coin-sol/src/sol.ts
Outdated
| // Calculate fee: lamportsPerSignature * numSignatures + (rent * numATAs) | ||
| const lamportsPerSignature = parseInt(params.feeInfo?.fee || '0', 10); | ||
| const rentPerAta = parseInt(params.tokenAccountRentExemptAmount || '0', 10); |
There was a problem hiding this comment.
@claude suggest a inner func that has a nicer signature with already parsed args
don't we want bigint for amounts anyway?
There was a problem hiding this comment.
🤖 Claude here.
Yes, bigint for amounts. The ExplainTransactionOptions.feeInfo.fee and tokenAccountRentExemptAmount come in as string from the external interface (TransactionFee.fee is string in sdk-core), so we should convert exactly once at the boundary.
Proposal — parse external args at the top, then pass typed values into a pure inner function:
interface WasmExplainArgs {
combinedInstructions: InstructionParams[];
transactionType: TransactionType;
memo?: string;
nonce: string;
durableNonce?: DurableNonceParams;
numSignatures: number;
signatures: Uint8Array[];
lamportsPerSignature: bigint;
rentPerAta: bigint;
}
function buildWasmExplanation(args: WasmExplainArgs): SolLibTransactionExplanation {
const { outputs, tokenEnablements, outputAmount } = extractOutputsFromInstructions(args.combinedInstructions);
const signatureFee = args.lamportsPerSignature * BigInt(args.numSignatures);
const rentFee = args.rentPerAta * BigInt(tokenEnablements.length);
const totalFee = (signatureFee + rentFee).toString();
const txId = getTransactionId(args.signatures);
return {
displayOrder: [ ... ],
id: txId,
type: TransactionType[args.transactionType],
changeOutputs: [],
changeAmount: '0',
outputAmount: outputAmount.toString(),
outputs,
fee: { fee: totalFee, feeRate: Number(args.lamportsPerSignature) },
memo: args.memo,
blockhash: args.nonce,
durableNonce: args.durableNonce,
tokenEnablements,
};
}Then explainTransactionWithWasm becomes a thin adapter:
explainTransactionWithWasm(params: ExplainTransactionOptions): SolLibTransactionExplanation {
const txBytes = Buffer.from(params.txBase64, 'base64');
const wasmTx = WasmTransaction.fromBytes(txBytes);
const parsed = wasmParseTransaction(txBytes);
const { instructions, transactionType } = combineWasmInstructionsFromBytes(txBytes);
const memo = parsed.instructionsData.find((i) => i.type === 'Memo')?.memo;
return buildWasmExplanation({
combinedInstructions: instructions,
transactionType,
memo,
nonce: parsed.nonce,
durableNonce: parsed.durableNonce,
numSignatures: parsed.numSignatures,
signatures: wasmTx.signatures,
lamportsPerSignature: BigInt(params.feeInfo?.fee || '0'),
rentPerAta: BigInt(params.tokenAccountRentExemptAmount || '0'),
});
}String → bigint conversion happens once at the boundary, all arithmetic is native bigint, and .toString() only at the final return where the external interface requires string.
modules/sdk-coin-sol/src/sol.ts
Outdated
| const totalFee = (signatureFee + rentFee).toString(); | ||
|
|
||
| // Get transaction id from first signature (base58 encoded) or UNAVAILABLE | ||
| let txId = 'UNAVAILABLE'; |
There was a problem hiding this comment.
this needs to be a higher level const
There was a problem hiding this comment.
🤖 Claude here.
Agreed — should use the existing UNAVAILABLE_TEXT constant from lib/constants.ts instead of a string literal. Same constant is already used in WasmTransaction for the same purpose.
modules/sdk-coin-sol/src/sol.ts
Outdated
| private mapTransactionTypeToString(type: TransactionType): string { | ||
| switch (type) { | ||
| case TransactionType.Send: | ||
| return 'Send'; | ||
| case TransactionType.WalletInitialization: | ||
| return 'WalletInitialization'; | ||
| case TransactionType.StakingActivate: | ||
| return 'StakingActivate'; | ||
| case TransactionType.StakingDeactivate: | ||
| return 'StakingDeactivate'; | ||
| case TransactionType.StakingWithdraw: | ||
| return 'StakingWithdraw'; | ||
| case TransactionType.StakingDelegate: | ||
| return 'StakingDelegate'; | ||
| case TransactionType.StakingAuthorize: | ||
| return 'StakingAuthorize'; | ||
| case TransactionType.AssociatedTokenAccountInitialization: | ||
| return 'AssociatedTokenAccountInitialization'; | ||
| case TransactionType.CloseAssociatedTokenAccount: | ||
| return 'CloseAssociatedTokenAccount'; | ||
| case TransactionType.CustomTx: | ||
| return 'CustomTx'; | ||
| default: | ||
| return 'Send'; | ||
| } |
There was a problem hiding this comment.
this should not be a coin method
There was a problem hiding this comment.
🤖 Claude here.
Agreed. TransactionType is a numeric enum, so TypeScript's reverse mapping already gives us string names. The entire mapTransactionTypeToString method can be replaced with:
TransactionType[transactionType]This is exactly what the legacy Transaction class does at transaction.ts:645:
type: TransactionType[this.type].toString(),No switch statement or coin method needed. This should be a standalone expression, not a method on Sol.
3cb4054 to
009f956
Compare
c071f3c to
e1a4c9d
Compare
01ddba7 to
9400a94
Compare
Add a standalone `explainSolTransaction()` function that calls @bitgo/wasm-solana's `explainTransaction()` directly and resolves token names via `@bitgo/statics`. This is a thin adapter (~40 lines) that: - Converts wasm-solana's bigint amounts to strings at the boundary - Resolves mint addresses to token names via @bitgo/statics - Maps to the existing TransactionExplanation interface No legacy code is modified. The adapter coexists alongside the existing Transaction class and builders. For tsol (testnet), explainTransaction() routes through the WASM adapter. For sol (mainnet), the legacy builder path is preserved. Changes: - sol.ts: add explainSolTransaction() + explainTransactionWithWasm() - iface.ts: add inputs, feePayer, ataOwnerMap to TransactionExplanation - instructionParamsFactory.ts: export findTokenName (used by adapter) - package.json: add @bitgo/wasm-solana dependency - webpack config: add WASM asset handling - New tests: Jito WASM verification + explain tests for all tx types BTC-3025 TICKET: BTC-0
9400a94 to
7d251f5
Compare
Route tsol transactions through the WASM-based explainSolTransaction() in Transaction.explainTransaction(), validating the WASM path against production traffic before replacing the legacy implementation for all networks. Changes: - transaction.ts: tsol uses explainSolTransaction() instead of the legacy switch/case block (no @solana/web3.js dependency) - explainTransactionWasm.ts: add StakingAuthorizeRaw type mapping and stakingAuthorize field support for full parity with legacy - explainTransactionWasm.ts: return fee '0' with feeRate undefined when feeInfo is not provided (matches downstream expectations) - Update test assertions for WASM output shape (ataOwnerMap, fee values, unresolvable token names) BTC-0 TICKET: BTC-0
6ced0df to
cd9bf88
Compare
cd9bf88 to
a59880d
Compare
Summary
Add a standalone
explainSolTransaction()function tosdk-coin-solthat uses@bitgo/wasm-solanato explain Solana transactions without depending on@solana/web3.js.This is a thin ~100-line adapter that replaces the need for the legacy 800+ line
Transactionclass explain logic. The heavy lifting (parsing, instruction combining, type derivation) happens in@bitgo/wasm-solana— this adapter only handles:sol:usdcvia@bitgo/statics)StakingAuthorize→StakingAuthorizeRawtype mapping for downstream compatibilityTransactionExplanationinterfaceChanges
explainTransactionWasm.ts(new): Standalone adapter that calls@bitgo/wasm-solana'sexplainTransaction(), resolves token names, maps staking authorize fields, and returnsTransactionExplanationincludinginputsandfeePayertransaction.ts: Route tsol through WASM-based explain (validates against production traffic before replacing legacy for all networks)sol.ts: AddexplainTransactionWithWasm()method at coin class leveliface.ts: AddataOwnerMapandstakingAuthorizetoTransactionExplanationinterfaceinstructionParamsFactory.ts: Add Jito stake pool operations (StakePoolDepositSol,StakePoolWithdrawStake)jitoWasmVerification.ts: New test file verifying Jito liquid staking explain outputtransaction.tstests: Updated assertions for WASM parity (fee defaults to '0' instead of 'UNAVAILABLE', ataOwnerMap, inputs, feePayer, token name via statics)sol.tstests: Updated explain assertions to includeinputsandfeePayerreturned by WASM pathwebpack config: Add WASM asset handling for@bitgo/wasm-solanapackage.json: Add@bitgo/wasm-solanadependencyKey decisions
tsol) to validate against production traffic before replacing the legacy path for mainnetfee: '0'andfeeRate: undefinedwhen fee info not provided (instead of'UNAVAILABLE'). Downstream already converts UNAVAILABLE → '0', so this is a no-op changeStakingAuthorizeRawtype name, so the adapter maps it@bitgo/statics. Tokens not in statics return the raw mint addressinputsandfeePayer— these fields were already on theTransactionExplanationinterface but weren't populated by the WASM path until nowDepends on
explainTransaction()withStakingAuthorizeInfo, optionallamportsPerSignature(defaults to 5000n)Test Plan
Transactionclass output for all transaction typesTICKET: BTC-3025