Skip to content

Commit 21585d6

Browse files
fix(sdk-coin-flrp): tighten fee/baseFee validation on C-chain imports
- Treat a zero fee/baseFee as unset instead of relying on string truthiness, and clear the inherited network txFee default in the import builder constructor so an unset fee is actually detectable. - Reject setting both fee and baseFee, since silently preferring baseFee masked the ambiguity. - Guard against a zero/negative computed fee in the baseFee path in case malformed UTXOs or upstream flarejs behavior changes slip through. - Rewrite the padding test to independently reconstruct the unpadded flarejs estimate and assert the padded fee is exactly 10% higher, instead of only checking determinism against a hard-coded floor. Ticket: CECHO-1821
1 parent 27ca6a4 commit 21585d6

2 files changed

Lines changed: 56 additions & 27 deletions

File tree

modules/sdk-coin-flrp/src/lib/ImportInCTxBuilder.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ const BASE_FEE_PADDING_BPS = 1000n;
2323
export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
2424
constructor(_coinConfig: Readonly<CoinConfig>) {
2525
super(_coinConfig);
26+
// Unlike P-chain-oriented atomic txs, a C-chain import has no meaningful default fee.
27+
// Clear the inherited network txFee default so an unset fee is detectable at build time,
28+
// rather than silently building with that unrelated default value.
29+
this.transaction._fee.fee = '';
2630
}
2731

2832
/**
@@ -133,9 +137,16 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
133137
if (this.transaction._to.length !== 1) {
134138
throw new BuildTransactionError('to is required');
135139
}
136-
if (!this.transaction._fee.fee && !this.transaction._fee.baseFee) {
140+
const hasFee = !!this.transaction._fee.fee && BigInt(this.transaction._fee.fee) !== BigInt(0);
141+
const hasBaseFee = !!this.transaction._fee.baseFee && BigInt(this.transaction._fee.baseFee) !== BigInt(0);
142+
if (!hasFee && !hasBaseFee) {
137143
throw new BuildTransactionError('fee is required');
138144
}
145+
if (hasFee && hasBaseFee) {
146+
throw new BuildTransactionError(
147+
'fee and baseFee are mutually exclusive: use baseFee for gas-aware fee calculation, or fee for a fixed amount, not both'
148+
);
149+
}
139150
if (!this.transaction._context) {
140151
throw new BuildTransactionError('context is required');
141152
}
@@ -168,12 +179,12 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
168179

169180
let importTx: UnsignedTx;
170181

171-
if (this.transaction._fee.baseFee) {
182+
if (hasBaseFee) {
172183
// Gas-aware path: let flarejs compute the actual import gas cost (including the
173184
// ~10,000 AtomicTxBaseCost) from the real tx size/inputs, instead of trusting a
174185
// pre-computed fee amount. Pad the base fee to absorb volatility between signing
175186
// and broadcast.
176-
const suppliedBaseFee = BigInt(this.transaction._fee.baseFee);
187+
const suppliedBaseFee = BigInt(this.transaction._fee.baseFee as string);
177188
const paddedBaseFee = (suppliedBaseFee * (10000n + BASE_FEE_PADDING_BPS)) / 10000n;
178189

179190
importTx = evm.newImportTxFromBaseFee(
@@ -187,7 +198,13 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
187198

188199
const innerImportTx = importTx.getTx() as evmSerial.ImportTx;
189200
const totalOutputAmount = innerImportTx.Outs.reduce((sum, out) => sum + out.amount.value(), BigInt(0));
190-
this.transaction._fee.fee = (totalUtxoAmount - totalOutputAmount).toString();
201+
const computedFee = totalUtxoAmount - totalOutputAmount;
202+
if (computedFee <= BigInt(0)) {
203+
throw new BuildTransactionError(
204+
`Computed import fee must be greater than 0, got ${computedFee.toString()} nFLR`
205+
);
206+
}
207+
this.transaction._fee.fee = computedFee.toString();
191208
} else {
192209
const actualFeeNFlr = BigInt(this.transaction._fee.fee);
193210

modules/sdk-coin-flrp/test/unit/lib/importInCTxBuilder.ts

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import assert from 'assert';
22
import 'should';
3-
import { TransactionBuilderFactory, Transaction } from '../../../src/lib';
3+
import { TransactionBuilderFactory, Transaction, Utils } from '../../../src/lib';
44
import { coins } from '@bitgo/statics';
55
import { IMPORT_IN_C as testData } from '../../resources/transactionData/importInC';
66
import { ON_CHAIN_TEST_WALLET } from '../../resources/account';
77
import signFlowTest from './signFlowTestSuit';
8-
import { secp256k1, UnsignedTx } from '@flarenetwork/flarejs';
8+
import { secp256k1, UnsignedTx, evm, evmSerial } from '@flarenetwork/flarejs';
99

1010
describe('Flrp Import In C Tx Builder', () => {
1111
const factory = new TransactionBuilderFactory(coins.get('tflrp'));
@@ -258,30 +258,42 @@ describe('Flrp Import In C Tx Builder', () => {
258258
);
259259
});
260260

261-
it('should pad the supplied base fee to absorb volatility between signing and broadcast', async () => {
261+
it('should pad the supplied base fee on top of the raw flarejs gas*baseFee estimate', async () => {
262262
const baseFeeWei = 500n;
263+
const fromAddress = ON_CHAIN_TEST_WALLET.user.pChainAddress;
264+
const toAddress = '0x96993BAEb6AaE2e06BF95F144e2775D4f8efbD35';
263265

264-
const buildWithBaseFee = async (fee: bigint) =>
265-
factory
266-
.getImportInCBuilder()
267-
.threshold(1)
268-
.locktime(0)
269-
.fromPubKey([ON_CHAIN_TEST_WALLET.user.pChainAddress])
270-
.to('0x96993BAEb6AaE2e06BF95F144e2775D4f8efbD35')
271-
.baseFee(fee)
272-
.decodedUtxos([utxo])
273-
.context(testData.context)
274-
.build();
275-
276-
const paddedTx = (await buildWithBaseFee(baseFeeWei)) as Transaction;
277-
const unpaddedFeeEquivalentTx = (await buildWithBaseFee(baseFeeWei)) as Transaction;
278-
279-
// Same input should be deterministic, and strictly greater than the raw baseFee * gas
280-
// (i.e. some padding is applied on top of the network-observed base fee).
281-
BigInt(paddedTx.fee.fee).should.equal(BigInt(unpaddedFeeEquivalentTx.fee.fee));
266+
const paddedTx = (await factory
267+
.getImportInCBuilder()
268+
.threshold(1)
269+
.locktime(0)
270+
.fromPubKey([fromAddress])
271+
.to(toAddress)
272+
.baseFee(baseFeeWei)
273+
.decodedUtxos([utxo])
274+
.context(testData.context)
275+
.build()) as Transaction;
282276
const paddedFee = BigInt(paddedTx.fee.fee);
283-
const minExpectedFee = baseFeeWei * 11300n; // > known real gas cost * baseFee, proving padding
284-
assert(paddedFee > minExpectedFee, `Expected paddedFee (${paddedFee}) to be above ${minExpectedFee}`);
277+
278+
// Reconstruct the same import tx directly via flarejs using the *unpadded* base fee,
279+
// to recover the network's raw gas*baseFee requirement with no buffer applied.
280+
const assetId = (coins.get('tflrp').network as unknown as { assetId: string }).assetId;
281+
const nativeUtxos = Utils.decodedToUtxos([utxo], assetId);
282+
const rawImportTx = evm.newImportTxFromBaseFee(
283+
testData.context,
284+
Utils.parseAddress(toAddress),
285+
[Utils.parseAddress(fromAddress)],
286+
nativeUtxos,
287+
'P',
288+
baseFeeWei
289+
) as UnsignedTx;
290+
const rawInnerTx = rawImportTx.getTx() as evmSerial.ImportTx;
291+
const rawTotalOutput = rawInnerTx.Outs.reduce((sum, out) => sum + out.amount.value(), BigInt(0));
292+
const rawFee = BigInt(utxo.amount) - rawTotalOutput;
293+
294+
assert(paddedFee > rawFee, `Expected padded fee (${paddedFee}) to exceed the unpadded raw fee (${rawFee})`);
295+
// Padding is exactly the configured 10% buffer applied to the base fee before estimation.
296+
assert.strictEqual(paddedFee, (rawFee * 11000n) / 10000n);
285297
});
286298
});
287299

0 commit comments

Comments
 (0)