From 8b58f284403159a54d9869c999e5406e3ccfa37e Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Fri, 3 Jan 2025 14:35:28 +0000 Subject: [PATCH 01/22] feat: added test for feature --- apps/docs/src/guide/errors/index.md | 6 ++ packages/errors/src/error-codes.ts | 2 + packages/fuel-gauge/src/transaction.test.ts | 66 ++++++++++++++++++++- 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/apps/docs/src/guide/errors/index.md b/apps/docs/src/guide/errors/index.md index 77772baa983..cfe101151b1 100644 --- a/apps/docs/src/guide/errors/index.md +++ b/apps/docs/src/guide/errors/index.md @@ -24,6 +24,12 @@ When an [`Account`](https://fuels-ts-docs-api.vercel.app/classes/_fuel_ts_accoun It could be caused during the deployments of contracts when an account is required to sign the transaction. This can be resolved by following the deployment guide [here](../contracts/deploying-contracts.md). +### `ASSET_BURN_DETECTED` + +When you are trying to send a transaction that will result in an asset burn. + +Add relevant coin change outputs to the transaction, or [enable asset burn](#TODO) in the transaction request. + ### `CONFIG_FILE_NOT_FOUND` When a configuration file is not found. This could either be a `fuels.config.[ts,js,mjs,cjs]` file or a TOML file. diff --git a/packages/errors/src/error-codes.ts b/packages/errors/src/error-codes.ts index 6728db60612..f4ec4436158 100644 --- a/packages/errors/src/error-codes.ts +++ b/packages/errors/src/error-codes.ts @@ -85,6 +85,8 @@ export enum ErrorCode { FUNDS_TOO_LOW = 'funds-too-low', MAX_OUTPUTS_EXCEEDED = 'max-outputs-exceeded', MAX_COINS_REACHED = 'max-coins-reached', + ASSET_BURN_DETECTED = 'asset-burn-detected', + // receipt INVALID_RECEIPT_TYPE = 'invalid-receipt-type', diff --git a/packages/fuel-gauge/src/transaction.test.ts b/packages/fuel-gauge/src/transaction.test.ts index 3befa770c10..4052de96eb8 100644 --- a/packages/fuel-gauge/src/transaction.test.ts +++ b/packages/fuel-gauge/src/transaction.test.ts @@ -1,5 +1,12 @@ -import { hexlify, InputMessageCoder, sleep, TransactionType } from 'fuels'; -import { launchTestNode, TestMessage } from 'fuels/test-utils'; +import { + FuelError, + hexlify, + InputMessageCoder, + ScriptTransactionRequest, + sleep, + TransactionType, +} from 'fuels'; +import { expectToThrowFuelError, launchTestNode, TestMessage } from 'fuels/test-utils'; import { CallTestContractFactory } from '../test/typegen'; @@ -139,4 +146,59 @@ describe('Transaction', () => { expect(isStatusSuccess).toBeTruthy(); expect(status.state).toBe('SPENT'); }); + + it('should allow an asset burn when enabled', async () => { + const { + provider, + wallets: [sender], + } = await launchTestNode(); + + const request = new ScriptTransactionRequest(); + + // Set the asset burn flag + request.enableBurn(true); + + // Add a coin input, without any output change + const baseAssetId = provider.getBaseAssetId(); + const { coins } = await sender.getCoins(baseAssetId); + const [coin] = coins; + request.addCoinInput(coin); + + const cost = await sender.getTransactionCost(request); + request.gasLimit = cost.gasUsed; + request.maxFee = cost.maxFee; + await sender.fund(request, cost); + + const { waitForResult } = await sender.sendTransaction(request); + const { isStatusSuccess } = await waitForResult(); + expect(isStatusSuccess).toEqual(true); + }); + + it('should throw an error when an asset burn is detected', async () => { + const { + provider, + wallets: [sender], + } = await launchTestNode(); + + const request = new ScriptTransactionRequest(); + + // Add a coin input, without any output change + const baseAssetId = provider.getBaseAssetId(); + const { coins } = await sender.getCoins(baseAssetId); + const [coin] = coins; + request.addCoinInput(coin); + + const cost = await sender.getTransactionCost(request); + request.gasLimit = cost.gasUsed; + request.maxFee = cost.maxFee; + await sender.fund(request, cost); + + await expectToThrowFuelError( + () => sender.sendTransaction(request), + new FuelError( + FuelError.CODES.ASSET_BURN_DETECTED, + 'Asset burn detected.\nAdd relevant coin change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' + ) + ); + }); }); From 0ea3d8fae586947a7fe72ee1e0e519860479e8bb Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Fri, 3 Jan 2025 17:32:52 +0000 Subject: [PATCH 02/22] feat: added guards against illicit asset burns --- .../account/src/providers/provider.test.ts | 20 +++++-- packages/account/src/providers/provider.ts | 7 +++ .../transaction-request.test.ts | 33 +++++++++++ .../transaction-request.ts | 28 ++++++++++ .../test/fixtures/transaction-request.ts | 55 +++++++++++-------- packages/fuel-gauge/src/transaction.test.ts | 39 +++++++------ 6 files changed, 139 insertions(+), 43 deletions(-) diff --git a/packages/account/src/providers/provider.test.ts b/packages/account/src/providers/provider.test.ts index d9775e3252d..8d89c431336 100644 --- a/packages/account/src/providers/provider.test.ts +++ b/packages/account/src/providers/provider.test.ts @@ -1,11 +1,11 @@ -import { Address } from '@fuel-ts/address'; +import { Address, getRandomB256 } from '@fuel-ts/address'; import { ZeroBytes32 } from '@fuel-ts/address/configs'; import { randomBytes, randomUUID } from '@fuel-ts/crypto'; import { FuelError, ErrorCode } from '@fuel-ts/errors'; import { expectToThrowFuelError, safeExec } from '@fuel-ts/errors/test-utils'; import { BN, bn } from '@fuel-ts/math'; import type { Receipt } from '@fuel-ts/transactions'; -import { InputType, ReceiptType } from '@fuel-ts/transactions'; +import { InputType, OutputType, ReceiptType } from '@fuel-ts/transactions'; import { DateTime, arrayify, sleep } from '@fuel-ts/utils'; import { ASSET_A, ASSET_B } from '@fuel-ts/utils/test-utils'; import { versions } from '@fuel-ts/versions'; @@ -34,7 +34,10 @@ import Provider, { } from './provider'; import type { ExcludeResourcesOption } from './resource'; import { isCoin } from './resource'; -import type { CoinTransactionRequestInput } from './transaction-request'; +import type { + ChangeTransactionRequestOutput, + CoinTransactionRequestInput, +} from './transaction-request'; import { CreateTransactionRequest, ScriptTransactionRequest } from './transaction-request'; import { TransactionResponse } from './transaction-response'; import type { SubmittedStatus } from './transaction-summary/types'; @@ -325,19 +328,27 @@ describe('Provider', () => { it('can call()', async () => { using launched = await setupTestProviderAndWallets(); const { provider } = launched; + const owner = getRandomB256(); const baseAssetId = provider.getBaseAssetId(); const CoinInputs: CoinTransactionRequestInput[] = [ { type: InputType.Coin, id: '0xbc90ada45d89ec6648f8304eaf8fa2b03384d3c0efabc192b849658f4689b9c500', - owner: baseAssetId, + owner, assetId: baseAssetId, txPointer: '0x00000000000000000000000000000000', amount: 500_000, witnessIndex: 0, }, ]; + const ChangeOutputs: ChangeTransactionRequestOutput[] = [ + { + type: OutputType.Change, + assetId: baseAssetId, + to: owner, + }, + ]; const transactionRequest = new ScriptTransactionRequest({ tip: 0, gasLimit: 100_000, @@ -352,6 +363,7 @@ describe('Provider', () => { arrayify('0x504000ca504400ba3341100024040000'), scriptData: randomBytes(32), inputs: CoinInputs, + outputs: ChangeOutputs, witnesses: ['0x'], }); diff --git a/packages/account/src/providers/provider.ts b/packages/account/src/providers/provider.ts index 5f9ffe88f42..f048cc5e8a3 100644 --- a/packages/account/src/providers/provider.ts +++ b/packages/account/src/providers/provider.ts @@ -866,6 +866,13 @@ Supported fuel-core version: ${supportedVersion}.` `The transaction exceeds the maximum allowed number of outputs. Tx outputs: ${tx.outputs.length}, max outputs: ${maxOutputs}` ); } + + if (tx.hasBurnableAssets()) { + throw new FuelError( + ErrorCode.ASSET_BURN_DETECTED, + 'Asset burn detected.\nAdd relevant coin change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' + ); + } } /** diff --git a/packages/account/src/providers/transaction-request/transaction-request.test.ts b/packages/account/src/providers/transaction-request/transaction-request.test.ts index abbbcd629e1..fb8b826defe 100644 --- a/packages/account/src/providers/transaction-request/transaction-request.test.ts +++ b/packages/account/src/providers/transaction-request/transaction-request.test.ts @@ -6,6 +6,10 @@ import { TransactionType, UpgradePurposeTypeEnum } from '@fuel-ts/transactions'; import { concat, hexlify } from '@fuel-ts/utils'; import { ASSET_A, ASSET_B } from '@fuel-ts/utils/test-utils'; +import { + SCRIPT_TX_COIN_REQUEST_INPUT, + SCRIPT_TX_COIN_REQUEST_OUTPUT_CHANGE, +} from '../../../test/fixtures/transaction-request'; import { WalletUnlocked } from '../../wallet'; import type { Coin } from '../coin'; import type { CoinQuantity } from '../coin-quantity'; @@ -291,4 +295,33 @@ describe('transactionRequestify', () => { expect(txRequest.witnessIndex).toEqual(txRequestLike.witnessIndex); expect(txRequest.type).toEqual(txRequestLike.type); }); + + it('should have burnable assets [single input, no change]', () => { + const txRequest = new ScriptTransactionRequest(); + const hasBurnableAssets = true; + + txRequest.inputs.push(SCRIPT_TX_COIN_REQUEST_INPUT); + + expect(txRequest.hasBurnableAssets()).toEqual(hasBurnableAssets); + }); + + it('should not have burnable assets [single input, single change]', () => { + const txRequest = new ScriptTransactionRequest(); + const hasBurnableAssets = false; + + txRequest.inputs.push(SCRIPT_TX_COIN_REQUEST_INPUT); + txRequest.outputs.push(SCRIPT_TX_COIN_REQUEST_OUTPUT_CHANGE); + + expect(txRequest.hasBurnableAssets()).toEqual(hasBurnableAssets); + }); + + it('should not have burnable assets [single input, burn asset enabled]', () => { + const txRequest = new ScriptTransactionRequest(); + const hasBurnableAssets = false; + + txRequest.enableBurn(true); + txRequest.inputs.push(SCRIPT_TX_COIN_REQUEST_INPUT); + + expect(txRequest.hasBurnableAssets()).toEqual(hasBurnableAssets); + }); }); diff --git a/packages/account/src/providers/transaction-request/transaction-request.ts b/packages/account/src/providers/transaction-request/transaction-request.ts index 9acc50532af..7f8b688761c 100644 --- a/packages/account/src/providers/transaction-request/transaction-request.ts +++ b/packages/account/src/providers/transaction-request/transaction-request.ts @@ -118,6 +118,8 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi outputs: TransactionRequestOutput[] = []; /** List of witnesses */ witnesses: TransactionRequestWitness[] = []; + /** Whether the transaction request should enable asset burn */ + burnEnabled: boolean = false; /** * Constructor for initializing a base transaction request. @@ -709,4 +711,30 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi byteLength(): number { return this.toTransactionBytes().byteLength; } + + /** + * Enables asset burn for the transaction request. + * + * @param burnEnabled - Whether the transaction request should enable asset burn. + * @returns This transaction request. + */ + enableBurn(burnEnabled: boolean = true): this { + this.burnEnabled = burnEnabled; + return this; + } + + hasBurnableAssets(): boolean { + if (this.burnEnabled) { + return false; + } + + const coinInputs = new Set(this.getCoinInputs().map((input) => input.assetId)); + const changeOutputs = new Set( + this.outputs + .filter((output) => output.type === OutputType.Change) + .map((output) => output.assetId) + ); + const difference = new Set([...coinInputs].filter((x) => !changeOutputs.has(x))); + return difference.size > 0; + } } diff --git a/packages/account/test/fixtures/transaction-request.ts b/packages/account/test/fixtures/transaction-request.ts index 9e9e7952532..ccc01cafc1c 100644 --- a/packages/account/test/fixtures/transaction-request.ts +++ b/packages/account/test/fixtures/transaction-request.ts @@ -1,5 +1,36 @@ +import type { + ChangeTransactionRequestOutput, + CoinTransactionRequestInput, + CoinTransactionRequestOutput, +} from '../../src'; import { ScriptTransactionRequest } from '../../src/providers/transaction-request/script-transaction-request'; +export const SCRIPT_TX_COIN_REQUEST_INPUT: CoinTransactionRequestInput = { + type: 0, + id: '0x000000000000000000000000000000000000000000000000000000000000000000', + assetId: '0x0000000000000000000000000000000000000000000000000000000000000000', + amount: '0x989680', + owner: '0xf1e92c42b90934aa6372e30bc568a326f6e66a1a0288595e6e3fbd392a4f3e6e', + txPointer: '0x00000000000000000000000000000000', + witnessIndex: 0, + predicate: '0x', + predicateData: '0x', + predicateGasUsed: '0x20', +}; + +export const SCRIPT_TX_COIN_REQUEST_OUTPUT_COIN: CoinTransactionRequestOutput = { + type: 0, + to: '0xc7862855b418ba8f58878db434b21053a61a2025209889cc115989e8040ff077', + assetId: '0x0000000000000000000000000000000000000000000000000000000000000000', + amount: 1, +}; + +export const SCRIPT_TX_COIN_REQUEST_OUTPUT_CHANGE: ChangeTransactionRequestOutput = { + type: 2, + to: '0xc7862855b418ba8f58878db434b21053a61a2025209889cc115989e8040ff077', + assetId: '0x0000000000000000000000000000000000000000000000000000000000000000', +}; + export const SCRIPT_TX_REQUEST = new ScriptTransactionRequest({ gasLimit: 10_000, script: '0x24400000', @@ -8,27 +39,7 @@ export const SCRIPT_TX_REQUEST = new ScriptTransactionRequest({ maxFee: 90000, maturity: 0, witnessLimit: 3000, - inputs: [ - { - type: 0, - id: '0x000000000000000000000000000000000000000000000000000000000000000000', - assetId: '0x0000000000000000000000000000000000000000000000000000000000000000', - amount: '0x989680', - owner: '0xf1e92c42b90934aa6372e30bc568a326f6e66a1a0288595e6e3fbd392a4f3e6e', - txPointer: '0x00000000000000000000000000000000', - witnessIndex: 0, - predicate: '0x', - predicateData: '0x', - predicateGasUsed: '0x20', - }, - ], - outputs: [ - { - type: 0, - to: '0xc7862855b418ba8f58878db434b21053a61a2025209889cc115989e8040ff077', - assetId: '0x0000000000000000000000000000000000000000000000000000000000000000', - amount: 1, - }, - ], + inputs: [SCRIPT_TX_COIN_REQUEST_INPUT], + outputs: [SCRIPT_TX_COIN_REQUEST_OUTPUT_COIN], witnesses: ['0x'], }); diff --git a/packages/fuel-gauge/src/transaction.test.ts b/packages/fuel-gauge/src/transaction.test.ts index 4052de96eb8..4604005dcde 100644 --- a/packages/fuel-gauge/src/transaction.test.ts +++ b/packages/fuel-gauge/src/transaction.test.ts @@ -1,12 +1,14 @@ +import type { CoinTransactionRequestInput } from 'fuels'; import { FuelError, hexlify, InputMessageCoder, + InputType, ScriptTransactionRequest, sleep, TransactionType, } from 'fuels'; -import { expectToThrowFuelError, launchTestNode, TestMessage } from 'fuels/test-utils'; +import { ASSET_A, expectToThrowFuelError, launchTestNode, TestMessage } from 'fuels/test-utils'; import { CallTestContractFactory } from '../test/typegen'; @@ -149,7 +151,6 @@ describe('Transaction', () => { it('should allow an asset burn when enabled', async () => { const { - provider, wallets: [sender], } = await launchTestNode(); @@ -158,9 +159,8 @@ describe('Transaction', () => { // Set the asset burn flag request.enableBurn(true); - // Add a coin input, without any output change - const baseAssetId = provider.getBaseAssetId(); - const { coins } = await sender.getCoins(baseAssetId); + // Add a coin input, which adds the relevant coin change output + const { coins } = await sender.getCoins(ASSET_A); const [coin] = coins; request.addCoinInput(coin); @@ -169,29 +169,34 @@ describe('Transaction', () => { request.maxFee = cost.maxFee; await sender.fund(request, cost); - const { waitForResult } = await sender.sendTransaction(request); - const { isStatusSuccess } = await waitForResult(); + const tx = await sender.sendTransaction(request); + const { isStatusSuccess } = await tx.waitForResult(); expect(isStatusSuccess).toEqual(true); }); - it('should throw an error when an asset burn is detected', async () => { + it.only('should throw an error when an asset burn is detected', async () => { const { - provider, wallets: [sender], } = await launchTestNode(); const request = new ScriptTransactionRequest(); // Add a coin input, without any output change - const baseAssetId = provider.getBaseAssetId(); - const { coins } = await sender.getCoins(baseAssetId); + const { coins } = await sender.getCoins(ASSET_A); const [coin] = coins; - request.addCoinInput(coin); - - const cost = await sender.getTransactionCost(request); - request.gasLimit = cost.gasUsed; - request.maxFee = cost.maxFee; - await sender.fund(request, cost); + const { id, owner, amount, assetId, predicate, predicateData } = coin; + const coinInput: CoinTransactionRequestInput = { + id, + type: InputType.Coin, + owner: owner.toB256(), + amount, + assetId, + txPointer: '0x00000000000000000000000000000000', + witnessIndex: request.getCoinInputWitnessIndexByOwner(owner) ?? request.addEmptyWitness(), + predicate, + predicateData, + }; + request.inputs.push(coinInput); await expectToThrowFuelError( () => sender.sendTransaction(request), From a33ec7291820b231653296964fd1412126ad3cb0 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Fri, 3 Jan 2025 17:33:35 +0000 Subject: [PATCH 03/22] chore: docs --- apps/docs/src/guide/errors/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/docs/src/guide/errors/index.md b/apps/docs/src/guide/errors/index.md index cfe101151b1..f28a91ef8b4 100644 --- a/apps/docs/src/guide/errors/index.md +++ b/apps/docs/src/guide/errors/index.md @@ -28,7 +28,7 @@ It could be caused during the deployments of contracts when an account is requir When you are trying to send a transaction that will result in an asset burn. -Add relevant coin change outputs to the transaction, or [enable asset burn](#TODO) in the transaction request. +Add relevant coin change outputs to the transaction, or enable asset burn in the transaction request. ### `CONFIG_FILE_NOT_FOUND` From 69e480fced47529d49986a90444c3d832d492760 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Fri, 3 Jan 2025 17:34:57 +0000 Subject: [PATCH 04/22] chore: changeset --- .changeset/wild-avocados-carry.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/wild-avocados-carry.md diff --git a/.changeset/wild-avocados-carry.md b/.changeset/wild-avocados-carry.md new file mode 100644 index 00000000000..2efc37c6c0b --- /dev/null +++ b/.changeset/wild-avocados-carry.md @@ -0,0 +1,6 @@ +--- +"@fuel-ts/account": patch +"@fuel-ts/errors": patch +--- + +feat: prevent implicit asset burn From 5e053b35ee82f99934cb75d7a582bcf38398b6a8 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Mon, 6 Jan 2025 10:02:37 +0000 Subject: [PATCH 05/22] chore: removed test.only --- packages/fuel-gauge/src/transaction.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/fuel-gauge/src/transaction.test.ts b/packages/fuel-gauge/src/transaction.test.ts index 4604005dcde..fef125304df 100644 --- a/packages/fuel-gauge/src/transaction.test.ts +++ b/packages/fuel-gauge/src/transaction.test.ts @@ -174,7 +174,7 @@ describe('Transaction', () => { expect(isStatusSuccess).toEqual(true); }); - it.only('should throw an error when an asset burn is detected', async () => { + it('should throw an error when an asset burn is detected', async () => { const { wallets: [sender], } = await launchTestNode(); From 61da932f12e313fdf986df219baa67793672073a Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Mon, 6 Jan 2025 10:09:09 +0000 Subject: [PATCH 06/22] chore: finalize the PR --- packages/account/src/providers/provider.ts | 2 +- .../providers/transaction-request/transaction-request.ts | 9 +++++++++ packages/fuel-gauge/src/transaction.test.ts | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/account/src/providers/provider.ts b/packages/account/src/providers/provider.ts index f048cc5e8a3..8e4397bf8e7 100644 --- a/packages/account/src/providers/provider.ts +++ b/packages/account/src/providers/provider.ts @@ -870,7 +870,7 @@ Supported fuel-core version: ${supportedVersion}.` if (tx.hasBurnableAssets()) { throw new FuelError( ErrorCode.ASSET_BURN_DETECTED, - 'Asset burn detected.\nAdd relevant coin change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' + 'Asset burn detected.\nAdd the relevant change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' ); } } diff --git a/packages/account/src/providers/transaction-request/transaction-request.ts b/packages/account/src/providers/transaction-request/transaction-request.ts index 7f8b688761c..1c6e8e337c8 100644 --- a/packages/account/src/providers/transaction-request/transaction-request.ts +++ b/packages/account/src/providers/transaction-request/transaction-request.ts @@ -713,6 +713,8 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi } /** + * WARNING: Do not enable without fully understanding the implications. + * * Enables asset burn for the transaction request. * * @param burnEnabled - Whether the transaction request should enable asset burn. @@ -723,6 +725,13 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi return this; } + /** + * @hidden + * + * Checks if the transaction request has burnable assets. + * + * @returns a boolean indicating whether the transaction request has burnable assets. + */ hasBurnableAssets(): boolean { if (this.burnEnabled) { return false; diff --git a/packages/fuel-gauge/src/transaction.test.ts b/packages/fuel-gauge/src/transaction.test.ts index fef125304df..0bc6bf6df33 100644 --- a/packages/fuel-gauge/src/transaction.test.ts +++ b/packages/fuel-gauge/src/transaction.test.ts @@ -202,7 +202,7 @@ describe('Transaction', () => { () => sender.sendTransaction(request), new FuelError( FuelError.CODES.ASSET_BURN_DETECTED, - 'Asset burn detected.\nAdd relevant coin change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' + 'Asset burn detected.\nAdd the relevant change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' ) ); }); From ac03f6470291396282ff4e1a61a256fec6b8526a Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 10:40:52 +0000 Subject: [PATCH 07/22] chore: refactored the burnable assets to a helper --- .../account/src/providers/provider.test.ts | 66 +++++++++++++++++++ packages/account/src/providers/provider.ts | 3 +- .../transaction-request/helpers.test.ts | 51 +++++++++++++- .../providers/transaction-request/helpers.ts | 16 ++++- .../transaction-request.test.ts | 29 -------- .../transaction-request.ts | 22 ------- 6 files changed, 133 insertions(+), 54 deletions(-) diff --git a/packages/account/src/providers/provider.test.ts b/packages/account/src/providers/provider.test.ts index 97397c3c8c6..64a65960391 100644 --- a/packages/account/src/providers/provider.test.ts +++ b/packages/account/src/providers/provider.test.ts @@ -2274,4 +2274,70 @@ Supported fuel-core version: ${mock.supportedVersion}.` expect(fetchChainAndNodeInfo).toHaveBeenCalledTimes(2); }); + + it('should throw error if asset burn is detected', async () => { + using launched = await setupTestProviderAndWallets(); + const { + wallets: [sender], + } = launched; + + const { + coins: [coin], + } = await sender.getCoins(ASSET_A); + + const request = new ScriptTransactionRequest(); + + // Add the coin as an input, without a change output + request.inputs.push({ + id: coin.id, + type: InputType.Coin, + owner: coin.owner.toB256(), + amount: coin.amount, + assetId: coin.assetId, + txPointer: '0x00000000000000000000000000000000', + witnessIndex: + request.getCoinInputWitnessIndexByOwner(coin.owner) ?? request.addEmptyWitness(), + }); + + await expectToThrowFuelError( + () => sender.sendTransaction(request), + new FuelError( + ErrorCode.ASSET_BURN_DETECTED, + 'Asset burn detected.\nAdd the relevant change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' + ) + ); + }); + + it('should allow asset burn if enabled', async () => { + using launched = await setupTestProviderAndWallets(); + const { + wallets: [sender], + } = launched; + const { + coins: [coin], + } = await sender.getCoins(ASSET_A); + + const request = new ScriptTransactionRequest(); + + // Enable asset burn + request.enableBurn(); + + // Add the coin as an input, without a change output + request.inputs.push({ + id: coin.id, + type: InputType.Coin, + owner: coin.owner.toB256(), + amount: coin.amount, + assetId: coin.assetId, + txPointer: '0x00000000000000000000000000000000', + witnessIndex: request.getCoinInputWitnessIndexByOwner(sender) ?? request.addEmptyWitness(), + }); + + // Fund the transaction + await request.autoCost(sender); + + const response = await sender.sendTransaction(request); + const { isStatusSuccess } = await response.waitForResult(); + expect(isStatusSuccess).toBe(true); + }); }); diff --git a/packages/account/src/providers/provider.ts b/packages/account/src/providers/provider.ts index 5b9b04e4a84..4a82515d593 100644 --- a/packages/account/src/providers/provider.ts +++ b/packages/account/src/providers/provider.ts @@ -46,6 +46,7 @@ import type { ScriptTransactionRequest, } from './transaction-request'; import { + getBurnableAssetCount, isTransactionTypeCreate, isTransactionTypeScript, transactionRequestify, @@ -852,7 +853,7 @@ Supported fuel-core version: ${supportedVersion}.` ); } - if (tx.hasBurnableAssets()) { + if (!tx.burnEnabled && getBurnableAssetCount(tx) > 0) { throw new FuelError( ErrorCode.ASSET_BURN_DETECTED, 'Asset burn detected.\nAdd the relevant change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' diff --git a/packages/account/src/providers/transaction-request/helpers.test.ts b/packages/account/src/providers/transaction-request/helpers.test.ts index 7759f1ac903..ea16e623fc2 100644 --- a/packages/account/src/providers/transaction-request/helpers.test.ts +++ b/packages/account/src/providers/transaction-request/helpers.test.ts @@ -1,7 +1,7 @@ import { getRandomB256, Address } from '@fuel-ts/address'; import { ZeroBytes32 } from '@fuel-ts/address/configs'; import { bn } from '@fuel-ts/math'; -import { InputType } from '@fuel-ts/transactions'; +import { InputType, OutputType } from '@fuel-ts/transactions'; import { generateFakeCoin, generateFakeMessageCoin } from '../../test-utils/resources'; import { @@ -20,7 +20,10 @@ import { getAssetAmountInRequestInputs, cacheRequestInputsResources, cacheRequestInputsResourcesFromOwner, + getBurnableAssetCount, } from './helpers'; +import type { TransactionRequestInput } from './input'; +import type { TransactionRequestOutput } from './output'; import { ScriptTransactionRequest } from './script-transaction-request'; /** @@ -196,5 +199,51 @@ describe('helpers', () => { expect(cached.messages).toStrictEqual([input3.nonce]); }); }); + + describe('getBurnableAssetCount', () => { + it('should get the number of burnable assets [0]', () => { + const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + ]; + const outputs: TransactionRequestOutput[] = [ + { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, + { type: OutputType.Change, assetId: ASSET_B, to: owner.toB256() }, + ]; + const expectedBurnableAssets = 0; + + const burnableAssets = getBurnableAssetCount({ inputs, outputs }); + + expect(burnableAssets).toBe(expectedBurnableAssets); + }); + + it('should get the number of burnable assets [1]', () => { + const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + ]; + const outputs: TransactionRequestOutput[] = [ + { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, + ]; + const expectedBurnableAssets = 1; + + const burnableAssets = getBurnableAssetCount({ inputs, outputs }); + + expect(burnableAssets).toBe(expectedBurnableAssets); + }); + + it('should get the number of burnable assets [2]', () => { + const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + ]; + const outputs: TransactionRequestOutput[] = []; + const expectedBurnableAssets = 2; + + const burnableAssets = getBurnableAssetCount({ inputs, outputs }); + + expect(burnableAssets).toBe(expectedBurnableAssets); + }); + }); }); }); diff --git a/packages/account/src/providers/transaction-request/helpers.ts b/packages/account/src/providers/transaction-request/helpers.ts index b66b670f86d..fca65722cb7 100644 --- a/packages/account/src/providers/transaction-request/helpers.ts +++ b/packages/account/src/providers/transaction-request/helpers.ts @@ -1,6 +1,6 @@ import type { AbstractAddress } from '@fuel-ts/interfaces'; import { bn } from '@fuel-ts/math'; -import { InputType } from '@fuel-ts/transactions'; +import { InputType, OutputType } from '@fuel-ts/transactions'; import type { ExcludeResourcesOption } from '../resource'; @@ -9,6 +9,7 @@ import type { CoinTransactionRequestInput, MessageTransactionRequestInput, } from './input'; +import type { TransactionRequestOutput } from './output'; export const isRequestInputCoin = ( input: TransactionRequestInput @@ -93,3 +94,16 @@ export const cacheRequestInputsResourcesFromOwner = ( messages: [], } as Required ); + +export const getBurnableAssetCount = (opts: { + inputs: TransactionRequestInput[]; + outputs: TransactionRequestOutput[]; +}) => { + const { inputs, outputs } = opts; + const coinInputs = new Set(inputs.filter(isRequestInputCoin).map((input) => input.assetId)); + const changeOutputs = new Set( + outputs.filter((output) => output.type === OutputType.Change).map((output) => output.assetId) + ); + const difference = new Set([...coinInputs].filter((x) => !changeOutputs.has(x))); + return difference.size; +}; diff --git a/packages/account/src/providers/transaction-request/transaction-request.test.ts b/packages/account/src/providers/transaction-request/transaction-request.test.ts index 1380c5e1506..fd7b9875f34 100644 --- a/packages/account/src/providers/transaction-request/transaction-request.test.ts +++ b/packages/account/src/providers/transaction-request/transaction-request.test.ts @@ -290,33 +290,4 @@ describe('transactionRequestify', () => { expect(txRequest.witnessIndex).toEqual(txRequestLike.witnessIndex); expect(txRequest.type).toEqual(txRequestLike.type); }); - - it('should have burnable assets [single input, no change]', () => { - const txRequest = new ScriptTransactionRequest(); - const hasBurnableAssets = true; - - txRequest.inputs.push(SCRIPT_TX_COIN_REQUEST_INPUT); - - expect(txRequest.hasBurnableAssets()).toEqual(hasBurnableAssets); - }); - - it('should not have burnable assets [single input, single change]', () => { - const txRequest = new ScriptTransactionRequest(); - const hasBurnableAssets = false; - - txRequest.inputs.push(SCRIPT_TX_COIN_REQUEST_INPUT); - txRequest.outputs.push(SCRIPT_TX_COIN_REQUEST_OUTPUT_CHANGE); - - expect(txRequest.hasBurnableAssets()).toEqual(hasBurnableAssets); - }); - - it('should not have burnable assets [single input, burn asset enabled]', () => { - const txRequest = new ScriptTransactionRequest(); - const hasBurnableAssets = false; - - txRequest.enableBurn(true); - txRequest.inputs.push(SCRIPT_TX_COIN_REQUEST_INPUT); - - expect(txRequest.hasBurnableAssets()).toEqual(hasBurnableAssets); - }); }); diff --git a/packages/account/src/providers/transaction-request/transaction-request.ts b/packages/account/src/providers/transaction-request/transaction-request.ts index 1c6e8e337c8..6be9fad54c2 100644 --- a/packages/account/src/providers/transaction-request/transaction-request.ts +++ b/packages/account/src/providers/transaction-request/transaction-request.ts @@ -724,26 +724,4 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi this.burnEnabled = burnEnabled; return this; } - - /** - * @hidden - * - * Checks if the transaction request has burnable assets. - * - * @returns a boolean indicating whether the transaction request has burnable assets. - */ - hasBurnableAssets(): boolean { - if (this.burnEnabled) { - return false; - } - - const coinInputs = new Set(this.getCoinInputs().map((input) => input.assetId)); - const changeOutputs = new Set( - this.outputs - .filter((output) => output.type === OutputType.Change) - .map((output) => output.assetId) - ); - const difference = new Set([...coinInputs].filter((x) => !changeOutputs.has(x))); - return difference.size > 0; - } } From 9c4080c4b3b3de1de4a1a3f1dd95271f4e57e6d5 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 10:41:08 +0000 Subject: [PATCH 08/22] chore: use `autoCost` --- packages/fuel-gauge/src/transaction.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/fuel-gauge/src/transaction.test.ts b/packages/fuel-gauge/src/transaction.test.ts index ac4c8757c05..e5f8f6e5358 100644 --- a/packages/fuel-gauge/src/transaction.test.ts +++ b/packages/fuel-gauge/src/transaction.test.ts @@ -154,10 +154,7 @@ describe('Transaction', () => { const [coin] = coins; request.addCoinInput(coin); - const cost = await sender.getTransactionCost(request); - request.gasLimit = cost.gasUsed; - request.maxFee = cost.maxFee; - await sender.fund(request, cost); + await request.autoCost(sender); const tx = await sender.sendTransaction(request); const { isStatusSuccess } = await tx.waitForResult(); From 16a6b07f0495b37a78036e4a22aa9bcbb8b8f68e Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 10:44:00 +0000 Subject: [PATCH 09/22] chore: breaking change --- .changeset/wild-avocados-carry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/wild-avocados-carry.md b/.changeset/wild-avocados-carry.md index 2efc37c6c0b..9356690542b 100644 --- a/.changeset/wild-avocados-carry.md +++ b/.changeset/wild-avocados-carry.md @@ -3,4 +3,4 @@ "@fuel-ts/errors": patch --- -feat: prevent implicit asset burn +feat!: prevent implicit asset burn From 262bba4e8eb1b6e65fd241e77f923d848507ce99 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 10:50:21 +0000 Subject: [PATCH 10/22] docs: added docs on asset burn --- .../transaction-request/asset-burn.ts | 39 +++++++++++++++++++ .../guide/transactions/transaction-request.md | 8 ++++ 2 files changed, 47 insertions(+) create mode 100644 apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts diff --git a/apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts b/apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts new file mode 100644 index 00000000000..0ed91b5c2da --- /dev/null +++ b/apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts @@ -0,0 +1,39 @@ +import { InputType, Provider, ScriptTransactionRequest, Wallet } from 'fuels'; +import { ASSET_A } from 'fuels/test-utils'; + +import { LOCAL_NETWORK_URL, WALLET_PVT_KEY } from '../../../../env'; + +const provider = new Provider(LOCAL_NETWORK_URL); +const sender = Wallet.fromPrivateKey(WALLET_PVT_KEY, provider); + +// #region asset-burn +const transactionRequest = new ScriptTransactionRequest(); + +const { + coins: [coin], +} = await sender.getCoins(ASSET_A); + +// Add the coin as an input, without a change output +transactionRequest.inputs.push({ + id: coin.id, + type: InputType.Coin, + owner: coin.owner.toB256(), + amount: coin.amount, + assetId: coin.assetId, + txPointer: '0x00000000000000000000000000000000', + witnessIndex: + transactionRequest.getCoinInputWitnessIndexByOwner(coin.owner) ?? + transactionRequest.addEmptyWitness(), +}); + +// Enable asset burn +transactionRequest.enableBurn(); + +// Fund the transaction +await transactionRequest.autoCost(sender); + +const tx = await sender.sendTransaction(transactionRequest); +// #endregion asset-burn + +const { isStatusSuccess } = await tx.waitForResult(); +console.log('Transaction should have been successful', isStatusSuccess); diff --git a/apps/docs/src/guide/transactions/transaction-request.md b/apps/docs/src/guide/transactions/transaction-request.md index 121ee218485..ea2fc0b8846 100644 --- a/apps/docs/src/guide/transactions/transaction-request.md +++ b/apps/docs/src/guide/transactions/transaction-request.md @@ -97,3 +97,11 @@ The transaction ID is a SHA-256 hash of the entire transaction request. This can <<< @./snippets/transaction-request/add-witness.ts#transaction-request-11{ts:line-numbers} > **Note**: Any changes made to a transaction request will alter the transaction ID. Therefore, you should only get the transaction ID after all modifications have been made. + +### Burning assets + +Assets can be burn't as part of a transaction that has inputs without associated output change. The SDK validates against this behaviour so we need to implicitly enable this with the `enableBurn` method on the transaction request. + +<<< @./snippets/transaction-request/asset-burn.ts#asset-burn{ts:line-numbers} + +> **Note**: Burning assets is permanent and all assets burn't will be lost. Therefore, be mindful of the usage of this functionality. From 4ff6361ce42a71748006ab8bac0bf3143ef6ea57 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 10:53:33 +0000 Subject: [PATCH 11/22] speeling --- apps/docs/src/guide/transactions/transaction-request.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/docs/src/guide/transactions/transaction-request.md b/apps/docs/src/guide/transactions/transaction-request.md index ea2fc0b8846..4cc227fdea6 100644 --- a/apps/docs/src/guide/transactions/transaction-request.md +++ b/apps/docs/src/guide/transactions/transaction-request.md @@ -100,8 +100,8 @@ The transaction ID is a SHA-256 hash of the entire transaction request. This can ### Burning assets -Assets can be burn't as part of a transaction that has inputs without associated output change. The SDK validates against this behaviour so we need to implicitly enable this with the `enableBurn` method on the transaction request. +Assets can be burnt as part of a transaction that has inputs without associated output change. The SDK validates against this behaviour so we need to implicitly enable this with the `enableBurn` method on the transaction request. <<< @./snippets/transaction-request/asset-burn.ts#asset-burn{ts:line-numbers} -> **Note**: Burning assets is permanent and all assets burn't will be lost. Therefore, be mindful of the usage of this functionality. +> **Note**: Burning assets is permanent and all assets burnt will be lost. Therefore, be mindful of the usage of this functionality. From ad96a1a00799e04401943a0cf3cae4139c20dad7 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 10:55:32 +0000 Subject: [PATCH 12/22] chore: changeset --- .changeset/wild-avocados-carry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/wild-avocados-carry.md b/.changeset/wild-avocados-carry.md index 9356690542b..d8219db0480 100644 --- a/.changeset/wild-avocados-carry.md +++ b/.changeset/wild-avocados-carry.md @@ -1,5 +1,5 @@ --- -"@fuel-ts/account": patch +"@fuel-ts/account": minor "@fuel-ts/errors": patch --- From c408eda3770a6cb6c7687ebae3a9493fa9209499 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 11:14:56 +0000 Subject: [PATCH 13/22] lintfix --- .../providers/transaction-request/transaction-request.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/account/src/providers/transaction-request/transaction-request.test.ts b/packages/account/src/providers/transaction-request/transaction-request.test.ts index fd7b9875f34..073b4597c90 100644 --- a/packages/account/src/providers/transaction-request/transaction-request.test.ts +++ b/packages/account/src/providers/transaction-request/transaction-request.test.ts @@ -6,10 +6,6 @@ import { TransactionType, UpgradePurposeTypeEnum } from '@fuel-ts/transactions'; import { concat, hexlify } from '@fuel-ts/utils'; import { ASSET_A, ASSET_B } from '@fuel-ts/utils/test-utils'; -import { - SCRIPT_TX_COIN_REQUEST_INPUT, - SCRIPT_TX_COIN_REQUEST_OUTPUT_CHANGE, -} from '../../../test/fixtures/transaction-request'; import { WalletUnlocked } from '../../wallet'; import type { Coin } from '../coin'; import type { CoinQuantity } from '../coin-quantity'; From 4004a0a5f8c1b188845bee2fca4ddda9d40f7da5 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 13:29:29 +0000 Subject: [PATCH 14/22] chore: allow asset burn via `sendTransaction` options --- .../transaction-request/asset-burn.ts | 8 +++---- .../guide/transactions/transaction-request.md | 2 +- .../account/src/providers/provider.test.ts | 18 +++++++------- packages/account/src/providers/provider.ts | 20 ++++++++-------- .../providers/transaction-request/helpers.ts | 24 +++++++++++++++++++ .../transaction-request.ts | 15 ------------ .../src/wallet/base-wallet-unlocked.ts | 4 ++-- packages/fuel-gauge/src/transaction.test.ts | 17 ++++++------- 8 files changed, 60 insertions(+), 48 deletions(-) diff --git a/apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts b/apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts index 0ed91b5c2da..2bc30ea0996 100644 --- a/apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts +++ b/apps/docs/src/guide/transactions/snippets/transaction-request/asset-burn.ts @@ -26,13 +26,13 @@ transactionRequest.inputs.push({ transactionRequest.addEmptyWitness(), }); -// Enable asset burn -transactionRequest.enableBurn(); - // Fund the transaction await transactionRequest.autoCost(sender); -const tx = await sender.sendTransaction(transactionRequest); +// Send the transaction with asset burn enabled +const tx = await sender.sendTransaction(transactionRequest, { + enableAssetBurn: true, +}); // #endregion asset-burn const { isStatusSuccess } = await tx.waitForResult(); diff --git a/apps/docs/src/guide/transactions/transaction-request.md b/apps/docs/src/guide/transactions/transaction-request.md index 4cc227fdea6..51eccf0f93e 100644 --- a/apps/docs/src/guide/transactions/transaction-request.md +++ b/apps/docs/src/guide/transactions/transaction-request.md @@ -100,7 +100,7 @@ The transaction ID is a SHA-256 hash of the entire transaction request. This can ### Burning assets -Assets can be burnt as part of a transaction that has inputs without associated output change. The SDK validates against this behaviour so we need to implicitly enable this with the `enableBurn` method on the transaction request. +Assets can be burnt as part of a transaction that has inputs without associated output change. The SDK validates against this behavior, so we need to implicitly enable this by sending the transaction with the `enableAssetBurn` option set to `true`. <<< @./snippets/transaction-request/asset-burn.ts#asset-burn{ts:line-numbers} diff --git a/packages/account/src/providers/provider.test.ts b/packages/account/src/providers/provider.test.ts index c762cc25d93..63fc4ae7d54 100644 --- a/packages/account/src/providers/provider.test.ts +++ b/packages/account/src/providers/provider.test.ts @@ -2299,12 +2299,14 @@ Supported fuel-core version: ${mock.supportedVersion}.` request.getCoinInputWitnessIndexByOwner(coin.owner) ?? request.addEmptyWitness(), }); + const expectedErrorMessage = [ + 'Asset burn detected.', + 'Add the relevant change outputs to the transaction to avoid burning assets.', + 'Or enable asset burn, upon sending the transaction.', + ].join('\n'); await expectToThrowFuelError( () => sender.sendTransaction(request), - new FuelError( - ErrorCode.ASSET_BURN_DETECTED, - 'Asset burn detected.\nAdd the relevant change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' - ) + new FuelError(ErrorCode.ASSET_BURN_DETECTED, expectedErrorMessage) ); }); @@ -2319,9 +2321,6 @@ Supported fuel-core version: ${mock.supportedVersion}.` const request = new ScriptTransactionRequest(); - // Enable asset burn - request.enableBurn(); - // Add the coin as an input, without a change output request.inputs.push({ id: coin.id, @@ -2336,10 +2335,13 @@ Supported fuel-core version: ${mock.supportedVersion}.` // Fund the transaction await request.autoCost(sender); - const response = await sender.sendTransaction(request); + const response = await sender.sendTransaction(request, { + enableAssetBurn: true, + }); const { isStatusSuccess } = await response.waitForResult(); expect(isStatusSuccess).toBe(true); }); + it('submits transaction and awaits status [success]', async () => { using launched = await setupTestProviderAndWallets(); const { diff --git a/packages/account/src/providers/provider.ts b/packages/account/src/providers/provider.ts index 3d3147d293e..52cfabcd017 100644 --- a/packages/account/src/providers/provider.ts +++ b/packages/account/src/providers/provider.ts @@ -46,10 +46,10 @@ import type { ScriptTransactionRequest, } from './transaction-request'; import { - getBurnableAssetCount, isTransactionTypeCreate, isTransactionTypeScript, transactionRequestify, + validateTransactionForAssetBurn, } from './transaction-request'; import type { TransactionResult, TransactionResultReceipt } from './transaction-response'; import { TransactionResponse, getDecodedLogs } from './transaction-response'; @@ -364,7 +364,12 @@ export type ProviderCallParams = UTXOValidationParams & EstimateTransactionParam /** * Provider Send transaction params */ -export type ProviderSendTxParams = EstimateTransactionParams; +export type ProviderSendTxParams = EstimateTransactionParams & { + /** + * Whether to enable asset burn for the transaction. + */ + enableAssetBurn?: boolean; +}; /** * URL - Consensus Params mapping. @@ -840,13 +845,6 @@ Supported fuel-core version: ${supportedVersion}.` `The transaction exceeds the maximum allowed number of outputs. Tx outputs: ${tx.outputs.length}, max outputs: ${maxOutputs}` ); } - - if (!tx.burnEnabled && getBurnableAssetCount(tx) > 0) { - throw new FuelError( - ErrorCode.ASSET_BURN_DETECTED, - 'Asset burn detected.\nAdd the relevant change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' - ); - } } /** @@ -861,9 +859,11 @@ Supported fuel-core version: ${supportedVersion}.` */ async sendTransaction( transactionRequestLike: TransactionRequestLike, - { estimateTxDependencies = true }: ProviderSendTxParams = {} + { estimateTxDependencies = true, enableAssetBurn = false }: ProviderSendTxParams = {} ): Promise { const transactionRequest = transactionRequestify(transactionRequestLike); + validateTransactionForAssetBurn(transactionRequest, enableAssetBurn); + if (estimateTxDependencies) { await this.estimateTxDependencies(transactionRequest); } diff --git a/packages/account/src/providers/transaction-request/helpers.ts b/packages/account/src/providers/transaction-request/helpers.ts index fca65722cb7..0d2b8d19e04 100644 --- a/packages/account/src/providers/transaction-request/helpers.ts +++ b/packages/account/src/providers/transaction-request/helpers.ts @@ -1,3 +1,4 @@ +import { ErrorCode, FuelError } from '@fuel-ts/errors'; import type { AbstractAddress } from '@fuel-ts/interfaces'; import { bn } from '@fuel-ts/math'; import { InputType, OutputType } from '@fuel-ts/transactions'; @@ -10,6 +11,7 @@ import type { MessageTransactionRequestInput, } from './input'; import type { TransactionRequestOutput } from './output'; +import type { TransactionRequest } from './types'; export const isRequestInputCoin = ( input: TransactionRequestInput @@ -107,3 +109,25 @@ export const getBurnableAssetCount = (opts: { const difference = new Set([...coinInputs].filter((x) => !changeOutputs.has(x))); return difference.size; }; + +export const validateTransactionForAssetBurn = ( + tx: TransactionRequest, + enableAssetBurn: boolean = false +) => { + // Asset burn is enabled + if (enableAssetBurn) { + return; + } + + // No burnable assets detected + if (getBurnableAssetCount(tx) <= 0) { + return; + } + + const message = [ + 'Asset burn detected.', + 'Add the relevant change outputs to the transaction to avoid burning assets.', + 'Or enable asset burn, upon sending the transaction.', + ].join('\n'); + throw new FuelError(ErrorCode.ASSET_BURN_DETECTED, message); +}; diff --git a/packages/account/src/providers/transaction-request/transaction-request.ts b/packages/account/src/providers/transaction-request/transaction-request.ts index 6be9fad54c2..9acc50532af 100644 --- a/packages/account/src/providers/transaction-request/transaction-request.ts +++ b/packages/account/src/providers/transaction-request/transaction-request.ts @@ -118,8 +118,6 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi outputs: TransactionRequestOutput[] = []; /** List of witnesses */ witnesses: TransactionRequestWitness[] = []; - /** Whether the transaction request should enable asset burn */ - burnEnabled: boolean = false; /** * Constructor for initializing a base transaction request. @@ -711,17 +709,4 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi byteLength(): number { return this.toTransactionBytes().byteLength; } - - /** - * WARNING: Do not enable without fully understanding the implications. - * - * Enables asset burn for the transaction request. - * - * @param burnEnabled - Whether the transaction request should enable asset burn. - * @returns This transaction request. - */ - enableBurn(burnEnabled: boolean = true): this { - this.burnEnabled = burnEnabled; - return this; - } } diff --git a/packages/account/src/wallet/base-wallet-unlocked.ts b/packages/account/src/wallet/base-wallet-unlocked.ts index 3eb66e13437..a7cadd23119 100644 --- a/packages/account/src/wallet/base-wallet-unlocked.ts +++ b/packages/account/src/wallet/base-wallet-unlocked.ts @@ -112,7 +112,7 @@ export class BaseWalletUnlocked extends Account { */ override async sendTransaction( transactionRequestLike: TransactionRequestLike, - { estimateTxDependencies = false }: ProviderSendTxParams = {} + { estimateTxDependencies = false, enableAssetBurn = false }: ProviderSendTxParams = {} ): Promise { const transactionRequest = transactionRequestify(transactionRequestLike); if (estimateTxDependencies) { @@ -120,7 +120,7 @@ export class BaseWalletUnlocked extends Account { } return this.provider.sendTransaction( await this.populateTransactionWitnessesSignature(transactionRequest), - { estimateTxDependencies: false } + { estimateTxDependencies: false, enableAssetBurn } ); } diff --git a/packages/fuel-gauge/src/transaction.test.ts b/packages/fuel-gauge/src/transaction.test.ts index e5f8f6e5358..62e79cc5a5b 100644 --- a/packages/fuel-gauge/src/transaction.test.ts +++ b/packages/fuel-gauge/src/transaction.test.ts @@ -146,9 +146,6 @@ describe('Transaction', () => { const request = new ScriptTransactionRequest(); - // Set the asset burn flag - request.enableBurn(true); - // Add a coin input, which adds the relevant coin change output const { coins } = await sender.getCoins(ASSET_A); const [coin] = coins; @@ -156,7 +153,9 @@ describe('Transaction', () => { await request.autoCost(sender); - const tx = await sender.sendTransaction(request); + const tx = await sender.sendTransaction(request, { + enableAssetBurn: true, + }); const { isStatusSuccess } = await tx.waitForResult(); expect(isStatusSuccess).toEqual(true); }); @@ -185,12 +184,14 @@ describe('Transaction', () => { }; request.inputs.push(coinInput); + const expectedErrorMessage = [ + 'Asset burn detected.', + 'Add the relevant change outputs to the transaction to avoid burning assets.', + 'Or enable asset burn, upon sending the transaction.', + ].join('\n'); await expectToThrowFuelError( () => sender.sendTransaction(request), - new FuelError( - FuelError.CODES.ASSET_BURN_DETECTED, - 'Asset burn detected.\nAdd the relevant change outputs to the transaction, or enable asset burn in the transaction request (`request.enableBurn()`).' - ) + new FuelError(FuelError.CODES.ASSET_BURN_DETECTED, expectedErrorMessage) ); }); }); From c5a5b7ee37c026711aaa0482c651d2b7b509db82 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 15:36:02 +0000 Subject: [PATCH 15/22] From 6f6a1569cb2c2512c1194cd09b38024c9c6772e2 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 7 Jan 2025 18:03:14 +0000 Subject: [PATCH 16/22] Update apps/docs/src/guide/transactions/transaction-request.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nedim Salkić --- apps/docs/src/guide/transactions/transaction-request.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/docs/src/guide/transactions/transaction-request.md b/apps/docs/src/guide/transactions/transaction-request.md index 51eccf0f93e..f663b9d72da 100644 --- a/apps/docs/src/guide/transactions/transaction-request.md +++ b/apps/docs/src/guide/transactions/transaction-request.md @@ -100,7 +100,7 @@ The transaction ID is a SHA-256 hash of the entire transaction request. This can ### Burning assets -Assets can be burnt as part of a transaction that has inputs without associated output change. The SDK validates against this behavior, so we need to implicitly enable this by sending the transaction with the `enableAssetBurn` option set to `true`. +Assets can be burnt as part of a transaction that has inputs without associated output change. The SDK validates against this behavior, so we need to explicitly enable this by sending the transaction with the `enableAssetBurn` option set to `true`. <<< @./snippets/transaction-request/asset-burn.ts#asset-burn{ts:line-numbers} From 82f472d680af20c24a61b458f7da334733a4aacd Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Wed, 8 Jan 2025 13:31:57 +0000 Subject: [PATCH 17/22] chore: added asset burn validation to wallet --- .../account/src/providers/provider.test.ts | 9 +- packages/account/src/providers/provider.ts | 2 +- .../transaction-request/helpers.test.ts | 83 +++++++++++++++++++ .../providers/transaction-request/helpers.ts | 30 +++++-- .../src/wallet/base-wallet-unlocked.ts | 5 +- 5 files changed, 116 insertions(+), 13 deletions(-) diff --git a/packages/account/src/providers/provider.test.ts b/packages/account/src/providers/provider.test.ts index 63fc4ae7d54..ccd041782ad 100644 --- a/packages/account/src/providers/provider.test.ts +++ b/packages/account/src/providers/provider.test.ts @@ -2278,6 +2278,7 @@ Supported fuel-core version: ${mock.supportedVersion}.` it('should throw error if asset burn is detected', async () => { using launched = await setupTestProviderAndWallets(); const { + provider, wallets: [sender], } = launched; @@ -2305,7 +2306,7 @@ Supported fuel-core version: ${mock.supportedVersion}.` 'Or enable asset burn, upon sending the transaction.', ].join('\n'); await expectToThrowFuelError( - () => sender.sendTransaction(request), + () => provider.sendTransaction(request), new FuelError(ErrorCode.ASSET_BURN_DETECTED, expectedErrorMessage) ); }); @@ -2313,6 +2314,7 @@ Supported fuel-core version: ${mock.supportedVersion}.` it('should allow asset burn if enabled', async () => { using launched = await setupTestProviderAndWallets(); const { + provider, wallets: [sender], } = launched; const { @@ -2335,7 +2337,10 @@ Supported fuel-core version: ${mock.supportedVersion}.` // Fund the transaction await request.autoCost(sender); - const response = await sender.sendTransaction(request, { + const signedTransaction = await sender.signTransaction(request); + request.updateWitnessByOwner(sender.address, signedTransaction); + + const response = await provider.sendTransaction(request, { enableAssetBurn: true, }); const { isStatusSuccess } = await response.waitForResult(); diff --git a/packages/account/src/providers/provider.ts b/packages/account/src/providers/provider.ts index 52cfabcd017..097fcef32f1 100644 --- a/packages/account/src/providers/provider.ts +++ b/packages/account/src/providers/provider.ts @@ -859,7 +859,7 @@ Supported fuel-core version: ${supportedVersion}.` */ async sendTransaction( transactionRequestLike: TransactionRequestLike, - { estimateTxDependencies = true, enableAssetBurn = false }: ProviderSendTxParams = {} + { estimateTxDependencies = true, enableAssetBurn }: ProviderSendTxParams = {} ): Promise { const transactionRequest = transactionRequestify(transactionRequestLike); validateTransactionForAssetBurn(transactionRequest, enableAssetBurn); diff --git a/packages/account/src/providers/transaction-request/helpers.test.ts b/packages/account/src/providers/transaction-request/helpers.test.ts index ea16e623fc2..8e139fb91ac 100644 --- a/packages/account/src/providers/transaction-request/helpers.test.ts +++ b/packages/account/src/providers/transaction-request/helpers.test.ts @@ -1,5 +1,7 @@ import { getRandomB256, Address } from '@fuel-ts/address'; import { ZeroBytes32 } from '@fuel-ts/address/configs'; +import { ErrorCode, FuelError } from '@fuel-ts/errors'; +import { expectToThrowFuelError } from '@fuel-ts/errors/test-utils'; import { bn } from '@fuel-ts/math'; import { InputType, OutputType } from '@fuel-ts/transactions'; @@ -21,6 +23,7 @@ import { cacheRequestInputsResources, cacheRequestInputsResourcesFromOwner, getBurnableAssetCount, + validateTransactionForAssetBurn, } from './helpers'; import type { TransactionRequestInput } from './input'; import type { TransactionRequestOutput } from './output'; @@ -245,5 +248,85 @@ describe('helpers', () => { expect(burnableAssets).toBe(expectedBurnableAssets); }); }); + + describe('validateTransactionForAssetBurn', () => { + it('should successfully validate transactions without burnable assets [enableAssetBurn=false]', () => { + const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + ]; + const outputs: TransactionRequestOutput[] = [ + { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, + { type: OutputType.Change, assetId: ASSET_B, to: owner.toB256() }, + ]; + const enableAssetBurn = false; + + const burnableAssets = validateTransactionForAssetBurn( + { inputs, outputs }, + enableAssetBurn + ); + + expect(burnableAssets).toBeUndefined(); + }); + + it('should throw an error if transaction has burnable assets [enableAssetBurn=false]', async () => { + const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + ]; + const outputs: TransactionRequestOutput[] = [ + { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, + ]; + const enableAssetBurn = false; + + await expectToThrowFuelError( + () => validateTransactionForAssetBurn({ inputs, outputs }, enableAssetBurn), + new FuelError( + ErrorCode.ASSET_BURN_DETECTED, + [ + `Asset burn detected.`, + `Add the relevant change outputs to the transaction to avoid burning assets.`, + `Or enable asset burn, upon sending the transaction.`, + ].join('\n') + ) + ); + }); + + it('should successfully validate transactions with burnable assets [enableAssetBurn=true]', () => { + const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + ]; + const outputs: TransactionRequestOutput[] = []; + const enableAssetBurn = true; + + const burnableAssets = validateTransactionForAssetBurn( + { inputs, outputs }, + enableAssetBurn + ); + + expect(burnableAssets).toBeUndefined(); + }); + + it('should validate asset burn by default [enableAssetBurn=undefined]', async () => { + const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + ]; + const outputs: TransactionRequestOutput[] = []; + + await expectToThrowFuelError( + () => validateTransactionForAssetBurn({ inputs, outputs }), + new FuelError( + ErrorCode.ASSET_BURN_DETECTED, + [ + `Asset burn detected.`, + `Add the relevant change outputs to the transaction to avoid burning assets.`, + `Or enable asset burn, upon sending the transaction.`, + ].join('\n') + ) + ); + }); + }); }); }); diff --git a/packages/account/src/providers/transaction-request/helpers.ts b/packages/account/src/providers/transaction-request/helpers.ts index 0d2b8d19e04..5ce80120c1d 100644 --- a/packages/account/src/providers/transaction-request/helpers.ts +++ b/packages/account/src/providers/transaction-request/helpers.ts @@ -10,7 +10,6 @@ import type { CoinTransactionRequestInput, MessageTransactionRequestInput, } from './input'; -import type { TransactionRequestOutput } from './output'; import type { TransactionRequest } from './types'; export const isRequestInputCoin = ( @@ -97,10 +96,15 @@ export const cacheRequestInputsResourcesFromOwner = ( } as Required ); -export const getBurnableAssetCount = (opts: { - inputs: TransactionRequestInput[]; - outputs: TransactionRequestOutput[]; -}) => { +/** + * @hidden + * + * Get the number of burnable assets in the transaction request. + * + * @param opts - The transaction request to get the burnable asset count from. + * @returns The number of burnable assets in the transaction request. + */ +export const getBurnableAssetCount = (opts: Pick) => { const { inputs, outputs } = opts; const coinInputs = new Set(inputs.filter(isRequestInputCoin).map((input) => input.assetId)); const changeOutputs = new Set( @@ -110,17 +114,27 @@ export const getBurnableAssetCount = (opts: { return difference.size; }; +/** + * @hidden + * + * Validates the transaction request for asset burn. + * + * @param transactionRequest - The transaction request to validate. + * @param enableAssetBurn - Whether asset burn is enabled (default: false). + * + * @throws `FuelError` when an asset burn is detected and not enabled. + */ export const validateTransactionForAssetBurn = ( - tx: TransactionRequest, + transactionRequest: Pick, enableAssetBurn: boolean = false ) => { // Asset burn is enabled - if (enableAssetBurn) { + if (enableAssetBurn === true) { return; } // No burnable assets detected - if (getBurnableAssetCount(tx) <= 0) { + if (getBurnableAssetCount(transactionRequest) <= 0) { return; } diff --git a/packages/account/src/wallet/base-wallet-unlocked.ts b/packages/account/src/wallet/base-wallet-unlocked.ts index a7cadd23119..988553c863d 100644 --- a/packages/account/src/wallet/base-wallet-unlocked.ts +++ b/packages/account/src/wallet/base-wallet-unlocked.ts @@ -3,7 +3,7 @@ import type { BytesLike } from '@fuel-ts/interfaces'; import { hexlify } from '@fuel-ts/utils'; import { Account } from '../account'; -import { transactionRequestify } from '../providers'; +import { transactionRequestify, validateTransactionForAssetBurn } from '../providers'; import type { TransactionRequestLike, CallResult, @@ -112,9 +112,10 @@ export class BaseWalletUnlocked extends Account { */ override async sendTransaction( transactionRequestLike: TransactionRequestLike, - { estimateTxDependencies = false, enableAssetBurn = false }: ProviderSendTxParams = {} + { estimateTxDependencies = false, enableAssetBurn }: ProviderSendTxParams = {} ): Promise { const transactionRequest = transactionRequestify(transactionRequestLike); + validateTransactionForAssetBurn(transactionRequest, enableAssetBurn); if (estimateTxDependencies) { await this.provider.estimateTxDependencies(transactionRequest); } From 3eef2943bdde2e7fbdbf53408614e2698735edd9 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Wed, 8 Jan 2025 13:32:37 +0000 Subject: [PATCH 18/22] chore: update wallet-unlocked tx example to be valid --- packages/account/test/fixtures/wallet-unlocked.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/account/test/fixtures/wallet-unlocked.ts b/packages/account/test/fixtures/wallet-unlocked.ts index 069c1abf87b..b01b0fbe410 100644 --- a/packages/account/test/fixtures/wallet-unlocked.ts +++ b/packages/account/test/fixtures/wallet-unlocked.ts @@ -29,6 +29,11 @@ export const SCRIPT_TX_REQUEST = new ScriptTransactionRequest({ assetId: '0x0000000000000000000000000000000000000000000000000000000000000000', amount: 1, }, + { + type: 2, + to: '0xc7862855b418ba8f58878db434b21053a61a2025209889cc115989e8040ff077', + assetId: '0x0000000000000000000000000000000000000000000000000000000000000000', + }, ], witnesses: ['0x'], }); @@ -39,4 +44,4 @@ export const PUBLIC_KEY = export const ADDRESS = '0xf1e92c42b90934aa6372e30bc568a326f6e66a1a0288595e6e3fbd392a4f3e6e'; export const HASHED_TX = '0x48ee795d94ea9562a3dbb9979cb44bb3dfd341eb755c378b14a3cd6886189980'; export const SIGNED_TX = - '0xbcc4d4988bf698ce45406e71112470ace2a6136bddf7dc1df87bfcc13d6a712ca82fa109cb697d02b14c3a6123fa0e06f4741aa0ee76f86cb26afac38ac493cb'; + '0x77d03430d2d68853da9b1ebeb1be84a08ea3ea54a55a374b40063181ff8a3b502ca00b15becc1621f4f250e80a5cfb596647c2ef2d50296ff2b8e4f1120b9c9f'; From 68e3a6a97c455e14b4d74c3fd53b8f37e7afcd1e Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Wed, 8 Jan 2025 15:06:01 +0000 Subject: [PATCH 19/22] chore: update test assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nedim Salkić --- .../transaction-request/helpers.test.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/account/src/providers/transaction-request/helpers.test.ts b/packages/account/src/providers/transaction-request/helpers.test.ts index 8e139fb91ac..e4db3ed0068 100644 --- a/packages/account/src/providers/transaction-request/helpers.test.ts +++ b/packages/account/src/providers/transaction-request/helpers.test.ts @@ -261,12 +261,9 @@ describe('helpers', () => { ]; const enableAssetBurn = false; - const burnableAssets = validateTransactionForAssetBurn( - { inputs, outputs }, - enableAssetBurn - ); - - expect(burnableAssets).toBeUndefined(); + expect(() => + validateTransactionForAssetBurn({ inputs, outputs }, enableAssetBurn) + ).not.toThrow(); }); it('should throw an error if transaction has burnable assets [enableAssetBurn=false]', async () => { @@ -300,12 +297,9 @@ describe('helpers', () => { const outputs: TransactionRequestOutput[] = []; const enableAssetBurn = true; - const burnableAssets = validateTransactionForAssetBurn( - { inputs, outputs }, - enableAssetBurn - ); - - expect(burnableAssets).toBeUndefined(); + expect(() => + validateTransactionForAssetBurn({ inputs, outputs }, enableAssetBurn) + ).not.toThrow(); }); it('should validate asset burn by default [enableAssetBurn=undefined]', async () => { From b321ec8f8c9bdeb5c5bf6caf1e14c9b131985927 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Thu, 9 Jan 2025 15:31:35 +0000 Subject: [PATCH 20/22] chore: added asset burn validation for `MessageCoin` --- packages/account/src/providers/provider.ts | 6 +- .../transaction-request/helpers.test.ts | 37 +++-- .../providers/transaction-request/helpers.ts | 12 +- .../src/wallet/base-wallet-unlocked.ts | 6 +- packages/fuel-gauge/src/transaction.test.ts | 131 +++++++++++++++++- 5 files changed, 163 insertions(+), 29 deletions(-) diff --git a/packages/account/src/providers/provider.ts b/packages/account/src/providers/provider.ts index 860f8e52635..09f7b84fa99 100644 --- a/packages/account/src/providers/provider.ts +++ b/packages/account/src/providers/provider.ts @@ -862,7 +862,11 @@ Supported fuel-core version: ${supportedVersion}.` { estimateTxDependencies = true, enableAssetBurn }: ProviderSendTxParams = {} ): Promise { const transactionRequest = transactionRequestify(transactionRequestLike); - validateTransactionForAssetBurn(transactionRequest, enableAssetBurn); + validateTransactionForAssetBurn( + await this.getBaseAssetId(), + transactionRequest, + enableAssetBurn + ); if (estimateTxDependencies) { await this.estimateTxDependencies(transactionRequest); diff --git a/packages/account/src/providers/transaction-request/helpers.test.ts b/packages/account/src/providers/transaction-request/helpers.test.ts index e4db3ed0068..9ec6c602ab0 100644 --- a/packages/account/src/providers/transaction-request/helpers.test.ts +++ b/packages/account/src/providers/transaction-request/helpers.test.ts @@ -206,44 +206,34 @@ describe('helpers', () => { describe('getBurnableAssetCount', () => { it('should get the number of burnable assets [0]', () => { const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputMessage(), // Will be a `baseAssetId` generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; const outputs: TransactionRequestOutput[] = [ + { type: OutputType.Change, assetId: baseAssetId, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_B, to: owner.toB256() }, ]; const expectedBurnableAssets = 0; - const burnableAssets = getBurnableAssetCount({ inputs, outputs }); + const burnableAssets = getBurnableAssetCount(baseAssetId, { inputs, outputs }); expect(burnableAssets).toBe(expectedBurnableAssets); }); - it('should get the number of burnable assets [1]', () => { + it('should get the number of burnable coins [2]', () => { const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputMessage(), // Burnable asset - will be a `baseAssetId` generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), - generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), + generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), // Burnable asset ]; const outputs: TransactionRequestOutput[] = [ { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, ]; - const expectedBurnableAssets = 1; - - const burnableAssets = getBurnableAssetCount({ inputs, outputs }); - - expect(burnableAssets).toBe(expectedBurnableAssets); - }); - - it('should get the number of burnable assets [2]', () => { - const inputs: TransactionRequestInput[] = [ - generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), - generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), - ]; - const outputs: TransactionRequestOutput[] = []; const expectedBurnableAssets = 2; - const burnableAssets = getBurnableAssetCount({ inputs, outputs }); + const burnableAssets = getBurnableAssetCount(baseAssetId, { inputs, outputs }); expect(burnableAssets).toBe(expectedBurnableAssets); }); @@ -252,22 +242,25 @@ describe('helpers', () => { describe('validateTransactionForAssetBurn', () => { it('should successfully validate transactions without burnable assets [enableAssetBurn=false]', () => { const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; const outputs: TransactionRequestOutput[] = [ + { type: OutputType.Change, assetId: baseAssetId, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_B, to: owner.toB256() }, ]; const enableAssetBurn = false; expect(() => - validateTransactionForAssetBurn({ inputs, outputs }, enableAssetBurn) + validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }, enableAssetBurn) ).not.toThrow(); }); it('should throw an error if transaction has burnable assets [enableAssetBurn=false]', async () => { const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; @@ -277,7 +270,7 @@ describe('helpers', () => { const enableAssetBurn = false; await expectToThrowFuelError( - () => validateTransactionForAssetBurn({ inputs, outputs }, enableAssetBurn), + () => validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }, enableAssetBurn), new FuelError( ErrorCode.ASSET_BURN_DETECTED, [ @@ -291,6 +284,7 @@ describe('helpers', () => { it('should successfully validate transactions with burnable assets [enableAssetBurn=true]', () => { const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; @@ -298,19 +292,20 @@ describe('helpers', () => { const enableAssetBurn = true; expect(() => - validateTransactionForAssetBurn({ inputs, outputs }, enableAssetBurn) + validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }, enableAssetBurn) ).not.toThrow(); }); it('should validate asset burn by default [enableAssetBurn=undefined]', async () => { const inputs: TransactionRequestInput[] = [ + generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; const outputs: TransactionRequestOutput[] = []; await expectToThrowFuelError( - () => validateTransactionForAssetBurn({ inputs, outputs }), + () => validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }), new FuelError( ErrorCode.ASSET_BURN_DETECTED, [ diff --git a/packages/account/src/providers/transaction-request/helpers.ts b/packages/account/src/providers/transaction-request/helpers.ts index ea23d7f18ce..6fc68ce6114 100644 --- a/packages/account/src/providers/transaction-request/helpers.ts +++ b/packages/account/src/providers/transaction-request/helpers.ts @@ -104,9 +104,16 @@ export const cacheRequestInputsResourcesFromOwner = ( * @param opts - The transaction request to get the burnable asset count from. * @returns The number of burnable assets in the transaction request. */ -export const getBurnableAssetCount = (opts: Pick) => { +export const getBurnableAssetCount = ( + baseAssetId: string, + opts: Pick +) => { const { inputs, outputs } = opts; const coinInputs = new Set(inputs.filter(isRequestInputCoin).map((input) => input.assetId)); + // If there is a message input without data, we need to add the base asset to the set + if (inputs.some(isRequestInputMessageWithoutData)) { + coinInputs.add(baseAssetId); + } const changeOutputs = new Set( outputs.filter((output) => output.type === OutputType.Change).map((output) => output.assetId) ); @@ -125,6 +132,7 @@ export const getBurnableAssetCount = (opts: Pick, enableAssetBurn: boolean = false ) => { @@ -134,7 +142,7 @@ export const validateTransactionForAssetBurn = ( } // No burnable assets detected - if (getBurnableAssetCount(transactionRequest) <= 0) { + if (getBurnableAssetCount(baseAssetId, transactionRequest) <= 0) { return; } diff --git a/packages/account/src/wallet/base-wallet-unlocked.ts b/packages/account/src/wallet/base-wallet-unlocked.ts index 59ce117045c..da43273c127 100644 --- a/packages/account/src/wallet/base-wallet-unlocked.ts +++ b/packages/account/src/wallet/base-wallet-unlocked.ts @@ -115,7 +115,11 @@ export class BaseWalletUnlocked extends Account { { estimateTxDependencies = false, enableAssetBurn }: ProviderSendTxParams = {} ): Promise { const transactionRequest = transactionRequestify(transactionRequestLike); - validateTransactionForAssetBurn(transactionRequest, enableAssetBurn); + validateTransactionForAssetBurn( + await this.provider.getBaseAssetId(), + transactionRequest, + enableAssetBurn + ); if (estimateTxDependencies) { await this.provider.estimateTxDependencies(transactionRequest); } diff --git a/packages/fuel-gauge/src/transaction.test.ts b/packages/fuel-gauge/src/transaction.test.ts index 62e79cc5a5b..574a4aca767 100644 --- a/packages/fuel-gauge/src/transaction.test.ts +++ b/packages/fuel-gauge/src/transaction.test.ts @@ -1,9 +1,10 @@ -import type { CoinTransactionRequestInput } from 'fuels'; +import type { CoinTransactionRequestInput, MessageTransactionRequestInput } from 'fuels'; import { FuelError, hexlify, InputMessageCoder, InputType, + isMessage, ScriptTransactionRequest, sleep, TransactionType, @@ -139,7 +140,7 @@ describe('Transaction', () => { expect(status.state).toBe('SPENT'); }); - it('should allow an asset burn when enabled', async () => { + it('should allow an asset burn when enabled (Coin)', async () => { const { wallets: [sender], } = await launchTestNode(); @@ -149,7 +150,19 @@ describe('Transaction', () => { // Add a coin input, which adds the relevant coin change output const { coins } = await sender.getCoins(ASSET_A); const [coin] = coins; - request.addCoinInput(coin); + const { id, owner, amount, assetId, predicate, predicateData } = coin; + const coinInput: CoinTransactionRequestInput = { + id, + type: InputType.Coin, + owner: owner.toB256(), + amount, + assetId, + txPointer: '0x00000000000000000000000000000000', + witnessIndex: request.getCoinInputWitnessIndexByOwner(owner) ?? request.addEmptyWitness(), + predicate, + predicateData, + }; + request.inputs.push(coinInput); await request.autoCost(sender); @@ -158,9 +171,14 @@ describe('Transaction', () => { }); const { isStatusSuccess } = await tx.waitForResult(); expect(isStatusSuccess).toEqual(true); + expect(request.outputs).to.not.contain( + expect.objectContaining({ + assetId: coinInput.assetId, + }) + ); }); - it('should throw an error when an asset burn is detected', async () => { + it('should throw an error when an asset burn is detected (Coin)', async () => { const { wallets: [sender], } = await launchTestNode(); @@ -193,5 +211,110 @@ describe('Transaction', () => { () => sender.sendTransaction(request), new FuelError(FuelError.CODES.ASSET_BURN_DETECTED, expectedErrorMessage) ); + expect(request.outputs).to.not.contain( + expect.objectContaining({ + assetId: coinInput.assetId, + }) + ); + }); + + it('should allow an asset burn when enabled (MessageCoin)', async () => { + const testMessage = new TestMessage({ + amount: 100_000, + }); + + const { + provider, + wallets: [owner], + } = await launchTestNode({ + walletsConfig: { + amountPerCoin: 0, + messages: [testMessage], + }, + }); + + const request = new ScriptTransactionRequest(); + + const resources = await owner.getResourcesToSpend([ + { assetId: await provider.getBaseAssetId(), amount: 100 }, + ]); + const [message] = resources.filter((r) => isMessage(r)); + + // Add a message coin input, which adds the relevant coin change output + const { sender, recipient, amount, nonce } = message; + const coinInput: MessageTransactionRequestInput = { + type: InputType.Message, + sender: sender.toB256(), + recipient: recipient.toB256(), + amount, + nonce, + witnessIndex: request.getCoinInputWitnessIndexByOwner(owner) ?? request.addEmptyWitness(), + }; + request.inputs.push(coinInput); + + await request.autoCost(owner); + + const tx = await owner.sendTransaction(request, { + enableAssetBurn: true, + }); + const { isStatusSuccess } = await tx.waitForResult(); + expect(isStatusSuccess).toEqual(true); + expect(request.outputs).to.not.contain( + expect.objectContaining({ + assetId: await provider.getBaseAssetId(), + }) + ); + }); + + it('should throw an error when an asset burn is detected (MessageCoin)', async () => { + const testMessage = new TestMessage({ + amount: 100_000, + }); + + const { + provider, + wallets: [owner], + } = await launchTestNode({ + walletsConfig: { + amountPerCoin: 0, + messages: [testMessage], + }, + }); + + const request = new ScriptTransactionRequest(); + + const resources = await owner.getResourcesToSpend([ + { assetId: await provider.getBaseAssetId(), amount: 100 }, + ]); + const [message] = resources.filter((r) => isMessage(r)); + + // Add a message coin input, without any output change + const { sender, recipient, amount, nonce } = message; + const coinInput: MessageTransactionRequestInput = { + type: InputType.Message, + sender: sender.toB256(), + recipient: recipient.toB256(), + amount, + nonce, + witnessIndex: request.getCoinInputWitnessIndexByOwner(owner) ?? request.addEmptyWitness(), + }; + request.inputs.push(coinInput); + + await request.autoCost(owner); + + const expectedErrorMessage = [ + 'Asset burn detected.', + 'Add the relevant change outputs to the transaction to avoid burning assets.', + 'Or enable asset burn, upon sending the transaction.', + ].join('\n'); + await expectToThrowFuelError( + () => owner.sendTransaction(request), + new FuelError(FuelError.CODES.ASSET_BURN_DETECTED, expectedErrorMessage) + ); + expect(request.outputs).to.not.contain( + expect.objectContaining({ + assetId: await provider.getBaseAssetId(), + }) + ); }); }); From 62dc663ebc3c93dddfdd92030dad5e029947e05d Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Thu, 9 Jan 2025 20:22:27 +0000 Subject: [PATCH 21/22] chore: update validation check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sérgio Torres <30977845+Torres-ssf@users.noreply.github.com> --- packages/account/src/providers/transaction-request/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account/src/providers/transaction-request/helpers.ts b/packages/account/src/providers/transaction-request/helpers.ts index 6fc68ce6114..55e3f460595 100644 --- a/packages/account/src/providers/transaction-request/helpers.ts +++ b/packages/account/src/providers/transaction-request/helpers.ts @@ -111,7 +111,7 @@ export const getBurnableAssetCount = ( const { inputs, outputs } = opts; const coinInputs = new Set(inputs.filter(isRequestInputCoin).map((input) => input.assetId)); // If there is a message input without data, we need to add the base asset to the set - if (inputs.some(isRequestInputMessageWithoutData)) { + if (inputs.some((i) => isRequestInputMessage(i) && bn(i.amount).gt(0))) { coinInputs.add(baseAssetId); } const changeOutputs = new Set( From 33e5ea9af0d6469b509683adcf07b5c57f21f854 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Thu, 9 Jan 2025 20:36:31 +0000 Subject: [PATCH 22/22] chore: use transactionRequest for helpers --- .../transaction-request/helpers.test.ts | 48 +++++++++---------- .../providers/transaction-request/helpers.ts | 9 ++-- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/packages/account/src/providers/transaction-request/helpers.test.ts b/packages/account/src/providers/transaction-request/helpers.test.ts index 9ec6c602ab0..e4dc9cfc3da 100644 --- a/packages/account/src/providers/transaction-request/helpers.test.ts +++ b/packages/account/src/providers/transaction-request/helpers.test.ts @@ -25,8 +25,6 @@ import { getBurnableAssetCount, validateTransactionForAssetBurn, } from './helpers'; -import type { TransactionRequestInput } from './input'; -import type { TransactionRequestOutput } from './output'; import { ScriptTransactionRequest } from './script-transaction-request'; /** @@ -205,35 +203,35 @@ describe('helpers', () => { describe('getBurnableAssetCount', () => { it('should get the number of burnable assets [0]', () => { - const inputs: TransactionRequestInput[] = [ + const request = new ScriptTransactionRequest(); + request.inputs = [ generateFakeRequestInputMessage(), // Will be a `baseAssetId` generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; - const outputs: TransactionRequestOutput[] = [ + request.outputs = [ { type: OutputType.Change, assetId: baseAssetId, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_B, to: owner.toB256() }, ]; const expectedBurnableAssets = 0; - const burnableAssets = getBurnableAssetCount(baseAssetId, { inputs, outputs }); + const burnableAssets = getBurnableAssetCount(baseAssetId, request); expect(burnableAssets).toBe(expectedBurnableAssets); }); it('should get the number of burnable coins [2]', () => { - const inputs: TransactionRequestInput[] = [ + const request = new ScriptTransactionRequest(); + request.inputs = [ generateFakeRequestInputMessage(), // Burnable asset - will be a `baseAssetId` generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), // Burnable asset ]; - const outputs: TransactionRequestOutput[] = [ - { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, - ]; + request.outputs = [{ type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }]; const expectedBurnableAssets = 2; - const burnableAssets = getBurnableAssetCount(baseAssetId, { inputs, outputs }); + const burnableAssets = getBurnableAssetCount(baseAssetId, request); expect(burnableAssets).toBe(expectedBurnableAssets); }); @@ -241,12 +239,13 @@ describe('helpers', () => { describe('validateTransactionForAssetBurn', () => { it('should successfully validate transactions without burnable assets [enableAssetBurn=false]', () => { - const inputs: TransactionRequestInput[] = [ + const request = new ScriptTransactionRequest(); + request.inputs = [ generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; - const outputs: TransactionRequestOutput[] = [ + request.outputs = [ { type: OutputType.Change, assetId: baseAssetId, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, { type: OutputType.Change, assetId: ASSET_B, to: owner.toB256() }, @@ -254,23 +253,22 @@ describe('helpers', () => { const enableAssetBurn = false; expect(() => - validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }, enableAssetBurn) + validateTransactionForAssetBurn(baseAssetId, request, enableAssetBurn) ).not.toThrow(); }); it('should throw an error if transaction has burnable assets [enableAssetBurn=false]', async () => { - const inputs: TransactionRequestInput[] = [ + const request = new ScriptTransactionRequest(); + request.inputs = [ generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; - const outputs: TransactionRequestOutput[] = [ - { type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }, - ]; + request.outputs = [{ type: OutputType.Change, assetId: ASSET_A, to: owner.toB256() }]; const enableAssetBurn = false; await expectToThrowFuelError( - () => validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }, enableAssetBurn), + () => validateTransactionForAssetBurn(baseAssetId, request, enableAssetBurn), new FuelError( ErrorCode.ASSET_BURN_DETECTED, [ @@ -283,29 +281,31 @@ describe('helpers', () => { }); it('should successfully validate transactions with burnable assets [enableAssetBurn=true]', () => { - const inputs: TransactionRequestInput[] = [ + const request = new ScriptTransactionRequest(); + request.inputs = [ generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; - const outputs: TransactionRequestOutput[] = []; + request.outputs = []; const enableAssetBurn = true; expect(() => - validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }, enableAssetBurn) + validateTransactionForAssetBurn(baseAssetId, request, enableAssetBurn) ).not.toThrow(); }); it('should validate asset burn by default [enableAssetBurn=undefined]', async () => { - const inputs: TransactionRequestInput[] = [ + const request = new ScriptTransactionRequest(); + request.inputs = [ generateFakeRequestInputMessage(), generateFakeRequestInputCoin({ assetId: ASSET_A, owner: owner.toB256() }), generateFakeRequestInputCoin({ assetId: ASSET_B, owner: owner.toB256() }), ]; - const outputs: TransactionRequestOutput[] = []; + request.outputs = []; await expectToThrowFuelError( - () => validateTransactionForAssetBurn(baseAssetId, { inputs, outputs }), + () => validateTransactionForAssetBurn(baseAssetId, request), new FuelError( ErrorCode.ASSET_BURN_DETECTED, [ diff --git a/packages/account/src/providers/transaction-request/helpers.ts b/packages/account/src/providers/transaction-request/helpers.ts index 55e3f460595..175f2e75cc7 100644 --- a/packages/account/src/providers/transaction-request/helpers.ts +++ b/packages/account/src/providers/transaction-request/helpers.ts @@ -101,14 +101,15 @@ export const cacheRequestInputsResourcesFromOwner = ( * * Get the number of burnable assets in the transaction request. * - * @param opts - The transaction request to get the burnable asset count from. + * @param baseAssetId - The base asset ID. + * @param transactionRequest - The transaction request to get the burnable asset count from. * @returns The number of burnable assets in the transaction request. */ export const getBurnableAssetCount = ( baseAssetId: string, - opts: Pick + transactionRequest: TransactionRequest ) => { - const { inputs, outputs } = opts; + const { inputs, outputs } = transactionRequest; const coinInputs = new Set(inputs.filter(isRequestInputCoin).map((input) => input.assetId)); // If there is a message input without data, we need to add the base asset to the set if (inputs.some((i) => isRequestInputMessage(i) && bn(i.amount).gt(0))) { @@ -133,7 +134,7 @@ export const getBurnableAssetCount = ( */ export const validateTransactionForAssetBurn = ( baseAssetId: string, - transactionRequest: Pick, + transactionRequest: TransactionRequest, enableAssetBurn: boolean = false ) => { // Asset burn is enabled