From 53dafb1c6e5e02e47f805c111a8f348278101a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Torres?= <30977845+Torres-ssf@users.noreply.github.com> Date: Wed, 20 Dec 2023 05:46:24 -0300 Subject: [PATCH] fix: `TransactionRequest` funding (#1531) --- .changeset/sour-apples-prove.md | 6 + .../src/funding-transaction.test.ts | 187 ++++++++++++++++++ packages/providers/src/provider.ts | 2 +- .../transaction-request.test.ts | 72 +------ .../transaction-request.ts | 75 +++---- packages/wallet/src/account.test.ts | 10 +- packages/wallet/src/account.ts | 65 +++++- 7 files changed, 306 insertions(+), 111 deletions(-) create mode 100644 .changeset/sour-apples-prove.md create mode 100644 packages/fuel-gauge/src/funding-transaction.test.ts diff --git a/.changeset/sour-apples-prove.md b/.changeset/sour-apples-prove.md new file mode 100644 index 00000000000..348182ab0b5 --- /dev/null +++ b/.changeset/sour-apples-prove.md @@ -0,0 +1,6 @@ +--- +"@fuel-ts/providers": minor +"@fuel-ts/wallet": minor +--- + +fix Account fund and Provider fundWithFakeUtxos diff --git a/packages/fuel-gauge/src/funding-transaction.test.ts b/packages/fuel-gauge/src/funding-transaction.test.ts new file mode 100644 index 00000000000..344ab933c4e --- /dev/null +++ b/packages/fuel-gauge/src/funding-transaction.test.ts @@ -0,0 +1,187 @@ +import { seedTestWallet } from '@fuel-ts/wallet/test-utils'; +import type { Account, CoinTransactionRequestInput } from 'fuels'; +import { + FUEL_NETWORK_URL, + Provider, + BaseAssetId, + ScriptTransactionRequest, + Wallet, + bn, +} from 'fuels'; + +describe(__filename, () => { + let mainWallet: Account; + let provider: Provider; + beforeAll(async () => { + provider = await Provider.create(FUEL_NETWORK_URL); + mainWallet = Wallet.generate({ provider }); + await seedTestWallet(mainWallet, [[500_000, BaseAssetId]]); + }); + + const fundingTxWithMultipleUTXOs = async ({ + account, + totalAmount, + splitIn, + }: { + account: Account; + totalAmount: number; + splitIn: number; + }) => { + const request = new ScriptTransactionRequest({ + gasLimit: 1_000, + gasPrice: bn(10), + }); + + for (let i = 0; i < splitIn; i++) { + request.addCoinOutput(account.address, totalAmount / splitIn, BaseAssetId); + } + + const resources = await mainWallet.getResourcesToSpend([[totalAmount + 2_000, BaseAssetId]]); + request.addResources(resources); + + const tx = await mainWallet.sendTransaction(request); + await tx.waitForResult(); + }; + + it('should successfully fund a transaction request when it is not fully funded', async () => { + const sender = Wallet.generate({ provider }); + const receiver = Wallet.generate({ provider }); + + // 1500 splitted in 5 = 5 UTXOs of 300 each + await fundingTxWithMultipleUTXOs({ + account: sender, + totalAmount: 1500, + splitIn: 5, + }); + + // this will return one UTXO of 300, not enought to pay for the TX fees + const lowResources = await sender.getResourcesToSpend([[100, BaseAssetId]]); + + // confirm we only fetched 1 UTXO from the expected amount + expect(lowResources.length).toBe(1); + expect(lowResources[0].amount.toNumber()).toBe(300); + + const request = new ScriptTransactionRequest({ + gasLimit: 1_000, + gasPrice: bn(10), + }); + + const amountToTransfer = 300; + request.addCoinOutput(receiver.address, amountToTransfer, BaseAssetId); + + request.addResources(lowResources); + + const { maxFee, requiredQuantities } = await provider.getTransactionCost(request); + + // TX request already does NOT carries enough resources, it needs to be funded + expect(request.inputs.length).toBe(1); + expect(bn((request.inputs[0]).amount).toNumber()).toBe(300); + expect(maxFee.gt(300)).toBeTruthy(); + + const getResourcesToSpendSpy = jest.spyOn(sender, 'getResourcesToSpend'); + + await sender.fund(request, requiredQuantities, maxFee); + + const tx = await sender.sendTransaction(request); + + await tx.waitForResult(); + + // fund method should have been called to fetch the remaining UTXOs + expect(getResourcesToSpendSpy).toHaveBeenCalledTimes(1); + + const receiverBalance = await receiver.getBalance(BaseAssetId); + + expect(receiverBalance.toNumber()).toBe(amountToTransfer); + }); + + it('should not fund a transaction request when it is already funded', async () => { + const sender = Wallet.generate({ provider }); + const receiver = Wallet.generate({ provider }); + + // 2000 splitted in 2 = 2 UTXOs of 1000 each + await fundingTxWithMultipleUTXOs({ + account: sender, + totalAmount: 2000, + splitIn: 2, + }); + + // sender has 2 UTXOs for 1000 each, so it has enough resources to spend 1000 of BaseAssetId + const enoughtResources = await sender.getResourcesToSpend([[100, BaseAssetId]]); + + // confirm we only fetched 1 UTXO from the expected amount + expect(enoughtResources.length).toBe(1); + expect(enoughtResources[0].amount.toNumber()).toBe(1000); + + const request = new ScriptTransactionRequest({ + gasLimit: 1_000, + gasPrice: bn(10), + }); + + const amountToTransfer = 100; + + request.addCoinOutput(receiver.address, amountToTransfer, BaseAssetId); + request.addResources(enoughtResources); + + const { maxFee, requiredQuantities } = await provider.getTransactionCost(request); + + // TX request already carries enough resources, it does not need to be funded + expect(request.inputs.length).toBe(1); + expect(bn((request.inputs[0]).amount).toNumber()).toBe(1000); + expect(maxFee.lt(1000)).toBeTruthy(); + + const getResourcesToSpendSpy = jest.spyOn(sender, 'getResourcesToSpend'); + + await sender.fund(request, requiredQuantities, maxFee); + + const tx = await sender.sendTransaction(request); + + await tx.waitForResult(); + + // fund should not have been called since the TX request was already funded + expect(getResourcesToSpendSpy).toHaveBeenCalledTimes(0); + + const receiverBalance = await receiver.getBalance(BaseAssetId); + + expect(receiverBalance.toNumber()).toBe(amountToTransfer); + }); + + it('should fully fund a transaction when it is has no funds yet', async () => { + const sender = Wallet.generate({ provider }); + const receiver = Wallet.generate({ provider }); + + // 5000 splitted in 10 = 10 UTXOs of 500 each + await fundingTxWithMultipleUTXOs({ + account: sender, + totalAmount: 5000, + splitIn: 10, + }); + + const request = new ScriptTransactionRequest({ + gasLimit: 1_000, + gasPrice: bn(10), + }); + + const amountToTransfer = 1000; + request.addCoinOutput(receiver.address, amountToTransfer, BaseAssetId); + + const { maxFee, requiredQuantities } = await provider.getTransactionCost(request); + + // TX request does NOT carry any resources, it needs to be funded + expect(request.inputs.length).toBe(0); + + const getResourcesToSpendSpy = jest.spyOn(sender, 'getResourcesToSpend'); + + await sender.fund(request, requiredQuantities, maxFee); + + const tx = await sender.sendTransaction(request); + + await tx.waitForResult(); + + // fund method should have been called to fetch UTXOs + expect(getResourcesToSpendSpy).toHaveBeenCalledTimes(1); + + const receiverBalance = await receiver.getBalance(BaseAssetId); + + expect(receiverBalance.toNumber()).toBe(amountToTransfer); + }); +}); diff --git a/packages/providers/src/provider.ts b/packages/providers/src/provider.ts index eb0ac793da7..f85cfe3756a 100644 --- a/packages/providers/src/provider.ts +++ b/packages/providers/src/provider.ts @@ -871,7 +871,7 @@ export default class Provider { excludedIds?: ExcludeResourcesOption ): Promise { const excludeInput = { - messages: excludedIds?.messages?.map((id) => hexlify(id)) || [], + messages: excludedIds?.messages?.map((nonce) => hexlify(nonce)) || [], utxos: excludedIds?.utxos?.map((id) => hexlify(id)) || [], }; diff --git a/packages/providers/src/transaction-request/transaction-request.test.ts b/packages/providers/src/transaction-request/transaction-request.test.ts index 7b41e2e444c..31e3404cf40 100644 --- a/packages/providers/src/transaction-request/transaction-request.test.ts +++ b/packages/providers/src/transaction-request/transaction-request.test.ts @@ -1,16 +1,8 @@ import { Address } from '@fuel-ts/address'; import { BaseAssetId } from '@fuel-ts/address/configs'; import { bn, toNumber } from '@fuel-ts/math'; -import { InputType, OutputType, TransactionType } from '@fuel-ts/transactions'; - -import { - MOCK_REQUEST_CHANGE_OUTPUT, - MOCK_REQUEST_COIN_INPUT, - MOCK_REQUEST_COIN_OUTPUT, - MOCK_REQUEST_CONTRACT_INPUT, - MOCK_REQUEST_CONTRACT_OUTPUT, - MOCK_REQUEST_MESSAGE_INPUT, -} from '../../test/fixtures/inputs-and-outputs'; +import { TransactionType } from '@fuel-ts/transactions'; + import type { CoinQuantity } from '../coin-quantity'; import type { CoinTransactionRequestInput } from './input'; @@ -128,65 +120,5 @@ describe('TransactionRequest', () => { expect(inputB?.amount).toEqual(bn(300)); expect(inputBase?.amount).toEqual(bn(500)); }); - - it('should add BaseAssetId with amount bn(1) if not present in quantities', () => { - const transactionRequest = new ScriptTransactionRequest(); - - const quantities: CoinQuantity[] = [{ assetId: assetIdB, amount: bn(10) }]; - - transactionRequest.fundWithFakeUtxos(quantities); - - const baseAssetEntry = quantities.find((q) => q.assetId === BaseAssetId); - expect(baseAssetEntry).not.toBeNull(); - expect(baseAssetEntry?.amount).toEqual(bn(1)); - }); - - it('should not add BaseAssetId if it is already present in quantities', () => { - const transactionRequest = new ScriptTransactionRequest(); - - const quantities = [{ assetId: BaseAssetId, amount: bn(10) }]; - transactionRequest.fundWithFakeUtxos(quantities); - const baseAssetEntries = quantities.filter((q) => q.assetId === BaseAssetId); - expect(baseAssetEntries.length).toBe(1); - }); - - it('should filter inputs and outputs accordingly', () => { - const transactionRequest = new ScriptTransactionRequest(); - - transactionRequest.inputs = [ - MOCK_REQUEST_COIN_INPUT, - MOCK_REQUEST_MESSAGE_INPUT, - MOCK_REQUEST_CONTRACT_INPUT, - ]; - transactionRequest.outputs = [ - MOCK_REQUEST_COIN_OUTPUT, - MOCK_REQUEST_CONTRACT_OUTPUT, - MOCK_REQUEST_CHANGE_OUTPUT, - ]; - - transactionRequest.fundWithFakeUtxos([]); - - const contractInput = transactionRequest.inputs.find((i) => i.type === InputType.Contract); - const coinInput = transactionRequest.inputs.find((i) => i.type === InputType.Coin); - const messageInput = transactionRequest.inputs.find((i) => i.type === InputType.Message); - - // Contract inputs should not be filtered out - expect(contractInput).toBe(MOCK_REQUEST_CONTRACT_INPUT); - - // Coin and Message inputs should be filtered out - expect(coinInput).not.toBe(MOCK_REQUEST_COIN_INPUT); - expect(messageInput).not.toBe(MOCK_REQUEST_MESSAGE_INPUT); - - const coinOutput = transactionRequest.outputs.find((o) => o.type === OutputType.Coin); - const contractOutput = transactionRequest.outputs.find((o) => o.type === OutputType.Contract); - const changeOutput = transactionRequest.outputs.find((o) => o.type === OutputType.Change); - - // Coin and Contract outputs should not be filtered out - expect(coinOutput).toBe(MOCK_REQUEST_COIN_OUTPUT); - expect(contractOutput).toBe(MOCK_REQUEST_CONTRACT_OUTPUT); - - // Change output should be filtered out - expect(changeOutput).not.toBe(MOCK_REQUEST_CHANGE_OUTPUT); - }); }); }); diff --git a/packages/providers/src/transaction-request/transaction-request.ts b/packages/providers/src/transaction-request/transaction-request.ts index 83092b4de8a..411814b434c 100644 --- a/packages/providers/src/transaction-request/transaction-request.ts +++ b/packages/providers/src/transaction-request/transaction-request.ts @@ -1,4 +1,4 @@ -import { Address, addressify, getRandomB256 } from '@fuel-ts/address'; +import { Address, addressify } from '@fuel-ts/address'; import { BaseAssetId, ZeroBytes32 } from '@fuel-ts/address/configs'; import type { AddressLike, AbstractAddress, AbstractPredicate } from '@fuel-ts/interfaces'; import type { BN, BigNumberish } from '@fuel-ts/math'; @@ -25,7 +25,6 @@ import { isCoin } from '../resource'; import { normalizeJSON } from '../utils'; import { getMaxGas, getMinGas } from '../utils/gas'; -import type { CoinTransactionRequestOutput } from '.'; import { NoWitnessAtIndexError } from './errors'; import type { TransactionRequestInput, @@ -33,7 +32,11 @@ import type { MessageTransactionRequestInput, } from './input'; import { inputify } from './input'; -import type { TransactionRequestOutput, ChangeTransactionRequestOutput } from './output'; +import type { + TransactionRequestOutput, + ChangeTransactionRequestOutput, + CoinTransactionRequestOutput, +} from './output'; import { outputify } from './output'; import type { TransactionRequestWitness } from './witness'; import { witnessify } from './witness'; @@ -559,42 +562,44 @@ export abstract class BaseTransactionRequest implements BaseTransactionRequestLi * @param quantities - CoinQuantity Array. */ fundWithFakeUtxos(quantities: CoinQuantity[]) { - const hasBaseAssetId = quantities.some(({ assetId }) => assetId === BaseAssetId); - - if (!hasBaseAssetId) { - quantities.push({ assetId: BaseAssetId, amount: bn(1) }); - } - - const owner = getRandomB256(); + let idCounter = 0; + const generateId = (): string => { + const counterString = String(idCounter++); + const id = ZeroBytes32.slice(0, -counterString.length).concat(counterString); + return id; + }; - const witnessToRemove = this.inputs.reduce( - (acc, input) => { - if (input.type === InputType.Coin || input.type === InputType.Message) { - if (!acc[input.witnessIndex]) { - acc[input.witnessIndex] = true; - } + const findAssetInput = (assetId: string) => + this.inputs.find((input) => { + if ('assetId' in input) { + return input.assetId === assetId; } + return false; + }); - return acc; - }, - {} as Record - ); - - this.witnesses = this.witnesses.filter((_, idx) => !witnessToRemove[idx]); - this.inputs = this.inputs.filter((input) => input.type === InputType.Contract); - this.outputs = this.outputs.filter((output) => output.type !== OutputType.Change); - - const fakeResources = quantities.map(({ assetId, amount }, idx) => ({ - id: `${ZeroBytes32}0${idx}`, - amount, - assetId, - owner: Address.fromB256(owner), - maturity: 0, - blockCreated: bn(1), - txCreatedIdx: bn(1), - })); + const updateAssetInput = (assetId: string, quantity: BN) => { + const assetInput = findAssetInput(assetId); + + if (assetInput && 'assetId' in assetInput) { + assetInput.id = generateId(); + assetInput.amount = quantity; + } else { + this.addResources([ + { + id: generateId(), + amount: quantity, + assetId, + owner: Address.fromRandom(), + maturity: 0, + blockCreated: bn(1), + txCreatedIdx: bn(1), + }, + ]); + } + }; - this.addResources(fakeResources); + updateAssetInput(BaseAssetId, bn(100_000_000_000)); + quantities.forEach((q) => updateAssetInput(q.assetId, q.amount)); } /** diff --git a/packages/wallet/src/account.test.ts b/packages/wallet/src/account.test.ts index e8f4f985c55..7f126c6257e 100644 --- a/packages/wallet/src/account.test.ts +++ b/packages/wallet/src/account.test.ts @@ -238,9 +238,15 @@ describe('Account', () => { coinQuantities: quantities, }); - const expectedTotalResources = [quantities[0], { amount: bn(fee), assetId: BaseAssetId }]; + const expectedTotalResources = [ + { amount: bn(quantities[0].amount), assetId: quantities[0].assetId }, + { amount: bn(fee), assetId: BaseAssetId }, + ]; expect(getResourcesToSpendSpy).toBeCalledTimes(1); - expect(getResourcesToSpendSpy).toBeCalledWith(expectedTotalResources); + expect(getResourcesToSpendSpy).toBeCalledWith(expectedTotalResources, { + messages: [], + utxos: [], + }); expect(addResourcesSpy).toBeCalledTimes(1); expect(addResourcesSpy).toHaveBeenCalledWith(resourcesToSpend); diff --git a/packages/wallet/src/account.ts b/packages/wallet/src/account.ts index 5f008351ab9..5a5f0eebd30 100644 --- a/packages/wallet/src/account.ts +++ b/packages/wallet/src/account.ts @@ -222,8 +222,63 @@ export class Account extends AbstractAccount { coinQuantities, }); - const resources = await this.getResourcesToSpend(updatedQuantities); - request.addResources(resources); + const quantitiesDict: Record = {}; + + updatedQuantities.forEach(({ amount, assetId }) => { + quantitiesDict[assetId] = { + required: amount, + owned: bn(0), + }; + }); + + const cachedUtxos: BytesLike[] = []; + const cachedMessages: BytesLike[] = []; + + const owner = this.address.toB256(); + + request.inputs.forEach((input) => { + const isResource = 'amount' in input; + + if (isResource) { + const isCoin = 'owner' in input; + + if (isCoin) { + const assetId = String(input.assetId); + if (input.owner === owner && quantitiesDict[assetId]) { + const amount = bn(input.amount); + quantitiesDict[assetId].owned = quantitiesDict[assetId].owned.add(amount); + + // caching this utxo to avoid fetching it again if requests needs to be funded + cachedUtxos.push(input.id); + } + } else if (input.recipient === owner && input.amount && quantitiesDict[BaseAssetId]) { + quantitiesDict[BaseAssetId].owned = quantitiesDict[BaseAssetId].owned.add(input.amount); + + // caching this message to avoid fetching it again if requests needs to be funded + cachedMessages.push(input.nonce); + } + } + }); + + const missingQuantities: CoinQuantity[] = []; + Object.entries(quantitiesDict).forEach(([assetId, { owned, required }]) => { + if (owned.lt(required)) { + missingQuantities.push({ + assetId, + amount: required.sub(owned), + }); + } + }); + + const needsToBeFunded = missingQuantities.length; + + if (needsToBeFunded) { + const resources = await this.getResourcesToSpend(missingQuantities, { + messages: cachedMessages, + utxos: cachedUtxos, + }); + request.addResources(resources); + } } /** @@ -355,8 +410,12 @@ export class Account extends AbstractAccount { const params = { script, ...txParams }; const request = new ScriptTransactionRequest(params); + const forwardingQuantities = [{ amount: bn(amount), assetId: BaseAssetId }]; - const { requiredQuantities, maxFee } = await this.provider.getTransactionCost(request); + const { requiredQuantities, maxFee } = await this.provider.getTransactionCost( + request, + forwardingQuantities + ); await this.fund(request, requiredQuantities, maxFee);