Skip to content

Commit 937b0a0

Browse files
authored
Merge pull request #7954 from BitGo/SC-5030
fix(sdk-coin-trx): fix BANDWIDTH transaction signature mismatch
2 parents b72aca0 + 1872bb1 commit 937b0a0

10 files changed

Lines changed: 586 additions & 28 deletions

.iyarc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44
# - This CVE affects archive EXTRACTION (unpacking malicious symlinks/hardlinks)
55
# - Lerna only uses tar for PACKING
66
GHSA-8qq5-rm4j-mr97
7-
7+
GHSA-r6q2-hw4h-h46w

modules/sdk-coin-trx/src/lib/delegateResourceTxBuilder.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,25 @@ export class DelegateResourceTxBuilder extends ResourceManagementTxBuilder {
7474
* @returns {string} the delegate resource transaction raw data hex
7575
*/
7676
protected getResourceManagementTxRawDataHex(): string {
77-
const rawContract = {
77+
const rawContract: {
78+
ownerAddress: number[];
79+
receiverAddress: number[];
80+
balance: string;
81+
resource?: string;
82+
} = {
7883
ownerAddress: getByteArrayFromHexAddress(this._ownerAddress),
7984
receiverAddress: getByteArrayFromHexAddress(this._receiverAddress),
8085
balance: this._balance,
81-
resource: this._resource,
8286
};
87+
88+
// Only include resource if it's not BANDWIDTH (the default value = 0)
89+
// In protobuf3, default values are typically not encoded in the wire format.
90+
// TRON's node re-serializes transactions and omits default values,
91+
// so we must match that behavior to ensure consistent transaction hashes.
92+
if (this._resource !== 'BANDWIDTH') {
93+
rawContract.resource = this._resource;
94+
}
95+
8396
const delegateResourceContract = protocol.DelegateResourceContract.fromObject(rawContract);
8497
const delegateResourceContractBytes = protocol.DelegateResourceContract.encode(delegateResourceContract).finish();
8598
const txContract = {

modules/sdk-coin-trx/src/lib/freezeBalanceTxBuilder.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,23 @@ export class FreezeBalanceTxBuilder extends TransactionBuilder {
163163
* @returns {string} the freeze balance transaction raw data hex
164164
*/
165165
private getFreezeRawDataHex(): string {
166-
const rawContract = {
166+
const rawContract: {
167+
ownerAddress: number[];
168+
frozenBalance: string;
169+
resource?: string;
170+
} = {
167171
ownerAddress: getByteArrayFromHexAddress(this._ownerAddress),
168172
frozenBalance: this._frozenBalance,
169-
resource: this._resource,
170173
};
174+
175+
// Only include resource if it's not BANDWIDTH (the default value = 0)
176+
// In protobuf3, default values are typically not encoded in the wire format.
177+
// TRON's node re-serializes transactions and omits default values,
178+
// so we must match that behavior to ensure consistent transaction hashes.
179+
if (this._resource !== 'BANDWIDTH') {
180+
rawContract.resource = this._resource;
181+
}
182+
171183
const freezeContract = protocol.FreezeBalanceV2Contract.fromObject(rawContract);
172184
const freezeContractBytes = protocol.FreezeBalanceV2Contract.encode(freezeContract).finish();
173185
const txContract = {

modules/sdk-coin-trx/src/lib/undelegateResourceTxBuilder.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,25 @@ export class UndelegateResourceTxBuilder extends ResourceManagementTxBuilder {
7474
* @returns {string} the undelegate resource transaction raw data hex
7575
*/
7676
protected getResourceManagementTxRawDataHex(): string {
77-
const rawContract = {
77+
const rawContract: {
78+
ownerAddress: number[];
79+
receiverAddress: number[];
80+
balance: string;
81+
resource?: string;
82+
} = {
7883
ownerAddress: getByteArrayFromHexAddress(this._ownerAddress),
7984
receiverAddress: getByteArrayFromHexAddress(this._receiverAddress),
8085
balance: this._balance,
81-
resource: this._resource,
8286
};
87+
88+
// Only include resource if it's not BANDWIDTH (the default value = 0)
89+
// In protobuf3, default values are typically not encoded in the wire format.
90+
// TRON's node re-serializes transactions and omits default values,
91+
// so we must match that behavior to ensure consistent transaction hashes.
92+
if (this._resource !== 'BANDWIDTH') {
93+
rawContract.resource = this._resource;
94+
}
95+
8396
const undelegateResourceContract = protocol.UnDelegateResourceContract.fromObject(rawContract);
8497
const undelegateResourceContractBytes =
8598
protocol.UnDelegateResourceContract.encode(undelegateResourceContract).finish();

modules/sdk-coin-trx/src/lib/unfreezeBalanceTxBuilder.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,23 @@ export class UnfreezeBalanceTxBuilder extends TransactionBuilder {
156156
* @returns {string} the freeze balance transaction raw data hex
157157
*/
158158
private getUnfreezeRawDataHex(): string {
159-
const rawContract = {
159+
const rawContract: {
160+
ownerAddress: number[];
161+
unfreezeBalance: string;
162+
resource?: string;
163+
} = {
160164
ownerAddress: getByteArrayFromHexAddress(this._ownerAddress),
161165
unfreezeBalance: this._unfreezeBalance,
162-
resource: this._resource,
163166
};
167+
168+
// Only include resource if it's not BANDWIDTH (the default value = 0)
169+
// In protobuf3, default values are typically not encoded in the wire format.
170+
// TRON's node re-serializes transactions and omits default values,
171+
// so we must match that behavior to ensure consistent transaction hashes.
172+
if (this._resource !== 'BANDWIDTH') {
173+
rawContract.resource = this._resource;
174+
}
175+
164176
const unfreezeContract = protocol.UnfreezeBalanceV2Contract.fromObject(rawContract);
165177
const unfreezeContractBytes = protocol.UnfreezeBalanceV2Contract.encode(unfreezeContract).finish();
166178
const txContract = {

modules/sdk-coin-trx/src/lib/utils.ts

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -447,10 +447,6 @@ export function decodeFreezeBalanceV2Contract(base64: string): FreezeBalanceCont
447447
throw new UtilsError('Owner address does not exist in this freeze contract.');
448448
}
449449

450-
if (freezeContract.resource === undefined) {
451-
throw new UtilsError('Resource type does not exist in this freeze contract.');
452-
}
453-
454450
if (freezeContract.frozenBalance === undefined) {
455451
throw new UtilsError('Frozen balance does not exist in this freeze contract.');
456452
}
@@ -459,7 +455,12 @@ export function decodeFreezeBalanceV2Contract(base64: string): FreezeBalanceCont
459455
getByteArrayFromHexAddress(Buffer.from(freezeContract.ownerAddress, 'base64').toString('hex'))
460456
);
461457

462-
const resourceValue = freezeContract.resource === 'BANDWIDTH' ? TronResource.BANDWIDTH : TronResource.ENERGY;
458+
// In protobuf3, default values (BANDWIDTH = 0) may be omitted from serialization.
459+
// If resource is undefined, default to BANDWIDTH.
460+
const resourceValue =
461+
freezeContract.resource === undefined || freezeContract.resource === 'BANDWIDTH'
462+
? TronResource.BANDWIDTH
463+
: TronResource.ENERGY;
463464

464465
return [
465466
{
@@ -549,10 +550,6 @@ export function decodeUnfreezeBalanceV2Contract(base64: string): UnfreezeBalance
549550
throw new UtilsError('Owner address does not exist in this unfreeze contract.');
550551
}
551552

552-
if (unfreezeContract.resource === undefined) {
553-
throw new UtilsError('Resource type does not exist in this unfreeze contract.');
554-
}
555-
556553
if (unfreezeContract.unfreezeBalance === undefined) {
557554
throw new UtilsError('Unfreeze balance does not exist in this unfreeze contract.');
558555
}
@@ -562,9 +559,13 @@ export function decodeUnfreezeBalanceV2Contract(base64: string): UnfreezeBalance
562559
getByteArrayFromHexAddress(Buffer.from(unfreezeContract.ownerAddress, 'base64').toString('hex'))
563560
);
564561

565-
// Convert ResourceCode enum value to string resource name
562+
// In protobuf3, default values (BANDWIDTH = 0) may be omitted from serialization.
563+
// If resource is undefined, default to BANDWIDTH.
566564
const resourceValue = unfreezeContract.resource;
567-
const resourceEnum = resourceValue === protocol.ResourceCode.BANDWIDTH ? TronResource.BANDWIDTH : TronResource.ENERGY;
565+
const resourceEnum =
566+
resourceValue === undefined || resourceValue === protocol.ResourceCode.BANDWIDTH
567+
? TronResource.BANDWIDTH
568+
: TronResource.ENERGY;
568569

569570
return [
570571
{
@@ -670,10 +671,6 @@ export function decodeDelegateResourceContract(base64: string): ResourceManageme
670671
throw new UtilsError('Receiver address does not exist in this delegate resource contract.');
671672
}
672673

673-
if (delegateResourceContract.resource === undefined) {
674-
throw new UtilsError('Resource type does not exist in this delegate resource contract.');
675-
}
676-
677674
if (delegateResourceContract.balance === undefined) {
678675
throw new UtilsError('Balance does not exist in this delegate resource contract.');
679676
}
@@ -686,6 +683,8 @@ export function decodeDelegateResourceContract(base64: string): ResourceManageme
686683
getByteArrayFromHexAddress(Buffer.from(delegateResourceContract.receiverAddress, 'base64').toString('hex'))
687684
);
688685

686+
// In protobuf3, default values (BANDWIDTH = 0) may be omitted from serialization.
687+
// If resource is undefined or falsy, default to BANDWIDTH.
689688
const resourceValue = !delegateResourceContract.resource ? TronResource.BANDWIDTH : TronResource.ENERGY;
690689

691690
return [
@@ -724,10 +723,6 @@ export function decodeUnDelegateResourceContract(base64: string): ResourceManage
724723
throw new UtilsError('Receiver address does not exist in this delegate resource contract.');
725724
}
726725

727-
if (undelegateResourceContract.resource === undefined) {
728-
throw new UtilsError('Resource type does not exist in this delegate resource contract.');
729-
}
730-
731726
if (undelegateResourceContract.balance === undefined) {
732727
throw new UtilsError('Balance does not exist in this delegate resource contract.');
733728
}
@@ -740,6 +735,8 @@ export function decodeUnDelegateResourceContract(base64: string): ResourceManage
740735
getByteArrayFromHexAddress(Buffer.from(undelegateResourceContract.receiverAddress, 'base64').toString('hex'))
741736
);
742737

738+
// In protobuf3, default values (BANDWIDTH = 0) may be omitted from serialization.
739+
// If resource is undefined or falsy, default to BANDWIDTH.
743740
const resourceValue = !undelegateResourceContract.resource ? TronResource.BANDWIDTH : TronResource.ENERGY;
744741

745742
return [

modules/sdk-coin-trx/test/unit/transactionBuilder/delegateResourceTxBuilder.ts

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
BLOCK_NUMBER,
99
EXPIRATION,
1010
RESOURCE_ENERGY,
11+
RESOURCE_BANDWIDTH,
1112
DELEGATION_BALANCE,
1213
DELEGATE_RESOURCE_CONTRACT,
1314
} from '../../resources';
@@ -323,4 +324,133 @@ describe('Tron DelegateResource builder', function () {
323324
});
324325
});
325326
});
327+
328+
describe('BANDWIDTH serialization (protobuf3 compatibility)', () => {
329+
it('should round-trip BANDWIDTH transactions correctly', async () => {
330+
// Build a BANDWIDTH transaction
331+
const builder = (getBuilder('ttrx') as WrappedBuilder).getDelegateResourceTxBuilder();
332+
builder
333+
.source({ address: PARTICIPANTS.from.address })
334+
.setReceiverAddress({ address: PARTICIPANTS.to.address })
335+
.block({ number: BLOCK_NUMBER, hash: BLOCK_HASH })
336+
.setBalance(DELEGATION_BALANCE)
337+
.setResource(RESOURCE_BANDWIDTH);
338+
339+
const tx = await builder.build();
340+
const txJson = tx.toJson();
341+
342+
// Verify the built transaction has BANDWIDTH resource
343+
assert.equal(
344+
txJson.raw_data.contract[0].parameter.value.resource,
345+
RESOURCE_BANDWIDTH,
346+
'Built transaction should have BANDWIDTH resource'
347+
);
348+
349+
// Round-trip: deserialize from broadcast format and rebuild
350+
const builder2 = getBuilder('ttrx').from(tx.toBroadcastFormat());
351+
const tx2 = await builder2.build();
352+
const tx2Json = tx2.toJson();
353+
354+
// Verify resource is preserved after round-trip
355+
assert.equal(
356+
tx2Json.raw_data.contract[0].parameter.value.resource,
357+
RESOURCE_BANDWIDTH,
358+
'Resource should be BANDWIDTH after round-trip from broadcast format'
359+
);
360+
361+
// Round-trip: deserialize from JSON and rebuild
362+
const builder3 = getBuilder('ttrx').from(tx.toJson());
363+
const tx3 = await builder3.build();
364+
const tx3Json = tx3.toJson();
365+
366+
// Verify resource is preserved after round-trip from JSON
367+
assert.equal(
368+
tx3Json.raw_data.contract[0].parameter.value.resource,
369+
RESOURCE_BANDWIDTH,
370+
'Resource should be BANDWIDTH after round-trip from JSON'
371+
);
372+
});
373+
374+
it('should round-trip ENERGY transactions correctly', async () => {
375+
// Build an ENERGY transaction
376+
const builder = (getBuilder('ttrx') as WrappedBuilder).getDelegateResourceTxBuilder();
377+
builder
378+
.source({ address: PARTICIPANTS.from.address })
379+
.setReceiverAddress({ address: PARTICIPANTS.to.address })
380+
.block({ number: BLOCK_NUMBER, hash: BLOCK_HASH })
381+
.setBalance(DELEGATION_BALANCE)
382+
.setResource(RESOURCE_ENERGY);
383+
384+
const tx = await builder.build();
385+
const txJson = tx.toJson();
386+
387+
// Verify the built transaction has ENERGY resource
388+
assert.equal(
389+
txJson.raw_data.contract[0].parameter.value.resource,
390+
RESOURCE_ENERGY,
391+
'Built transaction should have ENERGY resource'
392+
);
393+
394+
// Round-trip: deserialize from broadcast format and rebuild
395+
const builder2 = getBuilder('ttrx').from(tx.toBroadcastFormat());
396+
const tx2 = await builder2.build();
397+
const tx2Json = tx2.toJson();
398+
399+
// Verify resource is preserved after round-trip
400+
assert.equal(
401+
tx2Json.raw_data.contract[0].parameter.value.resource,
402+
RESOURCE_ENERGY,
403+
'Resource should be ENERGY after round-trip from broadcast format'
404+
);
405+
});
406+
407+
it('should produce consistent transaction IDs for BANDWIDTH transactions', async () => {
408+
// Build a BANDWIDTH transaction
409+
const builder = (getBuilder('ttrx') as WrappedBuilder).getDelegateResourceTxBuilder();
410+
builder
411+
.source({ address: PARTICIPANTS.from.address })
412+
.setReceiverAddress({ address: PARTICIPANTS.to.address })
413+
.block({ number: BLOCK_NUMBER, hash: BLOCK_HASH })
414+
.setBalance(DELEGATION_BALANCE)
415+
.setResource(RESOURCE_BANDWIDTH);
416+
417+
const tx = await builder.build();
418+
const originalTxId = tx.toJson().txID;
419+
420+
// Round-trip and verify txID is consistent
421+
const builder2 = getBuilder('ttrx').from(tx.toBroadcastFormat());
422+
const tx2 = await builder2.build();
423+
const roundTripTxId = tx2.toJson().txID;
424+
425+
assert.equal(originalTxId, roundTripTxId, 'Transaction ID should be consistent after round-trip');
426+
});
427+
428+
it('should allow signing BANDWIDTH transactions after round-trip', async () => {
429+
// Build a BANDWIDTH transaction
430+
const builder = (getBuilder('ttrx') as WrappedBuilder).getDelegateResourceTxBuilder();
431+
builder
432+
.source({ address: PARTICIPANTS.from.address })
433+
.setReceiverAddress({ address: PARTICIPANTS.to.address })
434+
.block({ number: BLOCK_NUMBER, hash: BLOCK_HASH })
435+
.setBalance(DELEGATION_BALANCE)
436+
.setResource(RESOURCE_BANDWIDTH);
437+
438+
const tx = await builder.build();
439+
440+
// Round-trip and sign
441+
const builder2 = getBuilder('ttrx').from(tx.toBroadcastFormat());
442+
builder2.sign({ key: PARTICIPANTS.from.pk });
443+
const signedTx = await builder2.build();
444+
445+
// Verify signature was added
446+
assert.equal(signedTx.toJson().signature.length, 1, 'Transaction should have one signature');
447+
448+
// Verify resource is still BANDWIDTH
449+
assert.equal(
450+
signedTx.toJson().raw_data.contract[0].parameter.value.resource,
451+
RESOURCE_BANDWIDTH,
452+
'Resource should still be BANDWIDTH after signing'
453+
);
454+
});
455+
});
326456
});

0 commit comments

Comments
 (0)