Skip to content

Commit d65632f

Browse files
Support Botbitgobot
authored andcommitted
fix(sdk-coin-eth): guard BN construction in signFinal for EIP-1559 txns
In Eth.signFinal (the second-signature path for WRW/offline-vault transactions), the ethTxParams object was unconditionally calling `new BN(txPrebuild.gasPrice)`. For EIP-1559 transactions, gasPrice is absent from txPrebuild, so bn.js received undefined and threw "Cannot read properties of undefined (reading 'toString')" internally. The buildTransaction helper already handles EIP-1559 correctly by consuming maxFeePerGas/maxPriorityFeePerGas from the eip1559 field and ignoring gasPrice when eip1559 is present, so passing undefined is safe. The fix skips BN construction when txPrebuild.eip1559 is set, preventing the crash when users raise fees on EIP-1559 Ethereum transactions. Added a WRWUnsignedSweepEIP1559ETHTx fixture and a corresponding test that exercises the isLastSignature=true path with an EIP-1559 txPrebuild (no gasPrice field). Ticket: COINS-572 Session-Id: afc68d99-c916-424d-bffc-d814dea1d611 Task-Id: 60985a53-c0de-4df0-9fb9-187bad6e4464
1 parent 389f90c commit d65632f

3 files changed

Lines changed: 54 additions & 1 deletion

File tree

modules/sdk-coin-eth/src/eth.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ export class Eth extends AbstractEthLikeNewCoins {
429429
nonce:
430430
params.signingKeyNonce !== undefined ? params.signingKeyNonce : params.txPrebuild.halfSigned?.backupKeyNonce,
431431
value: 0,
432-
gasPrice: new optionalDeps.ethUtil.BN(txPrebuild.gasPrice),
432+
gasPrice: txPrebuild.eip1559 ? undefined : new optionalDeps.ethUtil.BN(txPrebuild.gasPrice),
433433
gasLimit: new optionalDeps.ethUtil.BN(txPrebuild.gasLimit),
434434
data: sendData,
435435
};

modules/sdk-coin-eth/test/fixtures/eth.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,33 @@ module.exports.WRWUnsignedSweepETHTx = {
151151
nextContractSequenceId: 1,
152152
};
153153

154+
module.exports.WRWUnsignedSweepEIP1559ETHTx = {
155+
tx: 'f9012b808504a817c8008307a12094fd12f1d563650fbdd9314dce06992159778f634380b9010439125215000000000000000000000000a98fdfc2c711260cd665a3884b509b4a5ad6f4e80000000000000000000000000000000000000000000000003782dace9d90000000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000005f6a471c000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001c8080',
156+
userKey:
157+
'xpub661MyMwAqRbcFcNDKt46HgPAJTfNyQUS6M7i8jUwZKHz9wZGaK1XdQuT8XU5PkFfbrfoGXc1C4QD9PDJ7zhpu52rLzzynovwgcXh7NtDbH9',
158+
backupKey:
159+
'xpub661MyMwAqRbcGUJYcAgycBqG5HrQoUJAvBv7PEbvjGGffdtMP8hx3DX9AwzaY4vA7ynqHfxzRTRLwS2E9DH1HRPG8u7kWXd4JMCNgonGGnk',
160+
coin: 'teth',
161+
eip1559: { maxFeePerGas: '40000000000', maxPriorityFeePerGas: '2000000000' },
162+
gasLimit: '500000',
163+
recipients: [
164+
{
165+
address: '0xa98fdfc2c711260cd665a3884b509b4a5ad6f4e8',
166+
amount: '4000000000000000000',
167+
},
168+
],
169+
walletContractAddress: '0xfd12f1d563650fbdd9314dce06992159778f6343',
170+
amount: '4000000000000000000',
171+
backupKeyNonce: 0,
172+
recipient: {
173+
address: '0xa98fdfc2c711260cd665a3884b509b4a5ad6f4e8',
174+
amount: '4000000000000000000',
175+
},
176+
expireTime: 1600800540,
177+
contractSequenceId: 1,
178+
nextContractSequenceId: 1,
179+
};
180+
154181
module.exports.WRWUnsignedSweepERC20Tx = {
155182
tx: 'f9010a808504a817c8008307a12094df07117705a9f8dc4c2a78de66b7f1797dba9d4e80b8e40dcd7a6c00000000000000000000000052c8b29ab8b0a49a01c2b75f8e7f11b23e0e37820000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000004f96fe3b7a6cf9725f59d353f723c1bdb64ca6aa00000000000000000000000000000000000000000000000000000000611fb4330000000000000000000000000000000000000000000000000000000000002a7f00000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000001c8080',
156183
userKey:

modules/sdk-coin-eth/test/unit/ethWallet.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,4 +485,30 @@ describe('final-sign transaction from WRW', function () {
485485
outputs[0].address.should.equal(fixtures.WRWUnsignedSweepERC20Tx.recipient.address);
486486
outputs[0].amount.should.equal(fixtures.WRWUnsignedSweepERC20Tx.recipient.amount);
487487
});
488+
489+
it('should add a second EIP-1559 signature to unsigned sweep for teth without throwing when gasPrice is absent', async function () {
490+
const bitgo = TestBitGo.decorate(BitGoAPI, { env: 'test' });
491+
bitgo.safeRegister('teth', Teth.createInstance);
492+
const basecoin: any = bitgo.coin('teth');
493+
const gasLimit = 500000;
494+
const prv =
495+
'xprv9s21ZrQH143K3D8TXfvAJgHVfTEeQNW5Ys9wZtnUZkqPzFzSjbEJrWC1vZ4GnXCvR7rQL2UFX3RSuYeU9MrERm1XBvACow7c36vnz5iYyj2';
496+
const tx = {
497+
txPrebuild: fixtures.WRWUnsignedSweepEIP1559ETHTx,
498+
prv,
499+
};
500+
const halfSigned = await basecoin.signTransaction(tx);
501+
502+
const wrapper = {} as SignTransactionOptions;
503+
wrapper.txPrebuild = halfSigned;
504+
wrapper.txPrebuild.recipients = halfSigned.halfSigned.recipients;
505+
wrapper.txPrebuild.eip1559 = fixtures.WRWUnsignedSweepEIP1559ETHTx.eip1559;
506+
wrapper.txPrebuild.gasLimit = gasLimit.toString();
507+
wrapper.isLastSignature = true;
508+
wrapper.walletContractAddress = fixtures.WRWUnsignedSweepEIP1559ETHTx.walletContractAddress;
509+
wrapper.prv = prv;
510+
511+
const finalSignedTx = await basecoin.signTransaction(wrapper);
512+
finalSignedTx.should.have.property('txHex');
513+
});
488514
});

0 commit comments

Comments
 (0)