Skip to content

Commit

Permalink
checks max transaction size while funding transaction (#5687)
Browse files Browse the repository at this point in the history
keeps track of the posted size of a raw transaction in 'fund' and updates the
size after each spend is added to the raw transaction

throws an error if and when the posted size of the transaction will exceed the
maximum transaction size

inlines the logic of 'addSpendsForAsset' to avoid having to plumb posted size
and max transaction size down to that method

defines the constructor and error message for MaxTransactionSizeError
  • Loading branch information
hughy authored Dec 5, 2024
1 parent e10e565 commit 5ecdcef
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 59 deletions.
59 changes: 58 additions & 1 deletion ironfish/src/wallet/__fixtures__/wallet.test.ts.fixture
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@
"data": "base64:AQEAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAYCoHUBFmayr4IuahNwFYhfCAma32iBp26TRy8MSnkJKTHUZLbEw9bU9e+38IeLthr9WFlwHlaMxpiJI5PZh7Zeq7gyZLQLokHFoe4g3K1qCC0oSZORJzW1t/InNj5yQEffRuVUPWt7Povt+9BLkaQv4QRZV+VKChoYDlI6EAXYcR8ohRdVupTNaCrIe9PSq0KCQj59DHEfili/F9tPYO/0IQLF3dlOgujjgQxpZPCQCJnehM0USXKTRRD6P2gipystqO9/qE1CZl2XY2v+M5LdhrO2BxkKRUoNeIZh9lY0ByRvyOWLHQgS8/RpyD/aAEQJu5Qq+3qu9FtxTNKg/7Mb24QKH5L6UvbOKbcYlygZ7powVfTFLkJt6qiYYp+d4DBwAAANOWTcAmQNjUGDP/Uqan744/aooJtBSZ9DE4Lkc3UP7ZhZ4LuX2vb2x0pm1zoBWPQX4v2DdHQmG/FGpdHEmedbEnLpa0/lX9Mbn4ZceV+EE98Znx14EDPjfPTajK0+5BBbU9OBk1dvcG8872XHRGt2D3gniNebxNwB/ADezQGTWgT/6bJJEK5WYloMO+6ZFSBrn1B01TMOSIZzpgtbbKGyLu0LA6pohXOPbru5ajqy+Bv3aQwK5fwm1WNMPH+cJ6nRZpbSocviS2/92wg0CbYYBx92sgrIjvToOexZVIzeT8z4rxUXEsrC9h/COep8yuxIZJNRLT2hbWPiVEr998rj74+EmXvkrcswZS+TMG6YdQPpVSkRqBQDhJj107v8JHr1JYgk3MI+1z6UdXSU2CYmAmZjSv4MrMdaybUa4Sd98SK1M9K2mjm/z6NbcnDkhXXn4024V/A9lnCAQ2J0t39kck22+rgmPeU0RPDZ6UaOA1vNuK3Ln9UsoXfTmoZ8Iy7eOmMZl452jcxI+/U0oB77yyyNewH8ZVsbvfWFCrgABB/cVzdCxwW6SurFuj+Q6uTNY2NpN8AFxFdhm0yZGacKksq61JtfSosw0ioJh9T/nvGK0P44ViX9OWx0Yk6X4gj7pXsAtfg5x8AqRArlGZfmBHW1KxAVsi+E1KKJRb5eA+xQnhlqsUAMufaht/zyFaG2AJeIi+2M9Ew6gk9//bBA/O+dQnuEcgzc+7ZShjrUp/aiEkf3iyeHOIvaJly8tZ4+Fd5UC+aGHs0OZ4yxygQwdMClUb1fdMaFB3c1OJ0YNOZa33jmc/oWihmYZ/+YD8s/R82WKMtbsQ5pwkwZv2E+d6aVky9FVXQJMqk44vzk8mAYII5P9OZXOEChFI0d5k0uSjp/WeLWgOpzyL1KW4xvV4T5YUmykdV6inPSm1IMcB0RQTYe2ff0sGcOY5Y+0hrLJHnETwMFJtEH0by6w9RkQK7oAHxRnJw5eZZafDRmD9fwQD89wlabSsklTCPDBJp4TJvrdt2ibtmhZTLM8QTiFX1+1rn6m+804MFhxscu7FHVz//FY28/Xn1YwbRlJjg6lUhmPI5sDEA4JuzT6mLmET/5b901f2WkqmvgcCZVGDKDQYsPWMD5prFdkR2ALsvrakrNq8o1wKaY1MSwMAmyjpqxtoJ2qStXTAyQs/Z1pu2GIFJVDX346RleXECpUuxD0ovELqi44GpN9Z8swUNvMw6tUm1Synhy3igKrMy9j4oPzLttolJmKoJsC6iwscNVrhggru0qWXpWy4Jo4DBKdvUis5aYXX1pEIusRQXrPzG+UFDqcFYCd3xsTr/jcURpQjp0Pj9uptkGl13TgIqNMUV7DF/sNB2K5d7kMtMZZUOclfCURWYFdN2XQwgJbyfok3GhrQCXzSaBYncvK8J8apAnfyESeHHtDUKpf/qE+VHYIexya/BPCgfYeTiIGvm75UsrIoTytvQ3dcNIL7ZBb8h3BRJ7ABCCPtpLH/klZYlYtbJnN4l8bnA/SlWopD17YiBFSSbQ4fZ2HAYJs0r0xXGiDyyRPFS3TfexwntlWNOowaSvnkGOX5kUMOFQHzBQ=="
}
],
"Wallet addSpendsForAsset should select notes in order of largest to smallest": [
"Wallet fund should select notes in order of largest to smallest": [
{
"value": {
"version": 4,
Expand Down Expand Up @@ -8160,5 +8160,62 @@
}
]
}
],
"Wallet fund should throw error if transaction exceeds maximum size": [
{
"value": {
"encrypted": false,
"version": 4,
"id": "c9119687-458d-4274-b71f-083f3752237c",
"name": "a",
"spendingKey": "ef5421ccce3d6c0d25d6362d772557fef017600149ec9915d94e1139d3066db0",
"viewKey": "8a206def099a0fd98247242d4c95bc1011e27da6ac3c6de6450f1c51bb6ffc69bd48df4872375eef2f775644ce49098960ede02060b725cd2ed5b93a64a05241",
"incomingViewKey": "891143aca8864c452aeceaa173c80585ab2f0f7cff85b8e1902a3866ea432c03",
"outgoingViewKey": "f4999b8a20c254005fd198a741a0e719a08b78d3b882c5fbdd370e220513a3a5",
"publicAddress": "1d7843ed941dea3ab97358f6875d866dd55c00924b1563468765bbf43b227517",
"createdAt": {
"sequence": 1,
"hash": {
"type": "Buffer",
"data": "base64:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="
}
},
"scanningEnabled": true,
"proofAuthorizingKey": "5730da1768e4340e1429293d564c44ca93ea1ea966bd979153864caf6212890c"
},
"head": {
"hash": {
"type": "Buffer",
"data": "base64:R5HXrp+X3xAO8VWOhHctagm0N2I4goP3XG8goyqIqoY="
},
"sequence": 1
}
},
{
"header": {
"sequence": 2,
"previousBlockHash": "4791D7AE9F97DF100EF1558E84772D6A09B43762388283F75C6F20A32A88AA86",
"noteCommitment": {
"type": "Buffer",
"data": "base64:aohz3sMMBZ2FBlySAH34UomVCSASK6kkCNld4i2Fomo="
},
"transactionCommitment": {
"type": "Buffer",
"data": "base64:0T+kI35fIaOjufTPzTHaFY4gh2/c9ctr2UWVIm5PAU8="
},
"target": "9282972777491357380673661573939192202192629606981189395159182914949423",
"randomness": "0",
"timestamp": 1733269366090,
"graffiti": "0000000000000000000000000000000000000000000000000000000000000000",
"noteSize": 4,
"work": "0"
},
"transactions": [
{
"type": "Buffer",
"data": "base64:AQAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGzKiP////8AAAAA/EAemeXQYFvYVY9mhr+n6Uw7m0f1IBksYWeG0mmx5xCgmqBzzNHbsvIBf3FwG/FYmwmK2zuLIp7UWUEyUNDAuiRakHueZT5c/X+lZkx3TJStWON1uXWMhbMdJwGmZ+8dJWmKrDgB21MPYeag4dGlFnOWFFX5e/ZCaHZP5Fbmi/0AjmVTMRlytEqApJ2oq48FYsfrSEJwmL8trWr3HjuGs8+OoePjQipb34QQ8zSHZ4CsIPyJ7CbxXmEtpvtM6551t3hT/6o5bdz0UY6x7Vp2aumYYWx807Tmx81mkvcjk/ogGE4j9oQIdjCZA75gFy7BilGiu1No4wDQjVaXtfD/KooP5ddHGJ9twjKkvunXnaJBKQCZka2K1sZrQMyUGCBfrKUlY6Ce1paYsZYPHEswpKzS+jUTjvZ2vzbXOemuFeemXRaVV2lM3wwVHlBxBxL0FbJZCFRKgXfhDB/nFVBsRBD6VMk6OVRc4GBaRAKLoLludLIKhmY1Pqg61X8k80rB3UwJUTXNNUTepBrAwdlgLzMpFSefV51nvaovpJFSuT97xJAvAGtg+4LHAkyQdxd5v103sc6uxGf6+Tx7OkVpqs5250wePg7kvB+OQ94L4hAy0g6tDcnKWElyb24gRmlzaCBub3RlIGVuY3J5cHRpb24gbWluZXIga2V5MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwsvp63kXJYn5QIcnyo/aBjcUnQS7WNhCdgshbDX0JNAAHOynqc/iePuN1pLEBfO8SeGAKLReNV2Umigys03z3Aw=="
}
]
}
]
}
4 changes: 4 additions & 0 deletions ironfish/src/wallet/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export class MaxMemoLengthError extends Error {

export class MaxTransactionSizeError extends Error {
name = this.constructor.name
constructor(maxTransactionSize: number) {
super()
this.message = `Proposed transaction is larger than maximum transaction size of ${maxTransactionSize} bytes`
}
}

export class DuplicateAccountNameError extends Error {
Expand Down
59 changes: 47 additions & 12 deletions ironfish/src/wallet/wallet.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
import { Asset, generateKey, MEMO_LENGTH } from '@ironfish/rust-nodejs'
import { Asset, generateKey, MEMO_LENGTH, Note as NativeNote } from '@ironfish/rust-nodejs'
import { BufferMap, BufferSet } from 'buffer-map'
import { v4 as uuid } from 'uuid'
import { Assert } from '../assert'
import { Blockchain } from '../blockchain'
import { VerificationResultReason, Verifier } from '../consensus'
import { RawTransaction } from '../primitives'
import { Note, RawTransaction } from '../primitives'
import { TransactionVersion } from '../primitives/transaction'
import {
createNodeTest,
Expand Down Expand Up @@ -259,7 +259,7 @@ describe('Wallet', () => {
expect(await accountA.getNoteHash(forkSpendNullifier)).toBeUndefined()
})

describe('addSpendsForAsset', () => {
describe('fund', () => {
it('should select notes in order of largest to smallest', async () => {
const { node } = nodeTest
const accountA = await useAccountFixture(node.wallet, 'a')
Expand All @@ -275,21 +275,56 @@ describe('Wallet', () => {
await node.wallet.scan()

const rawTransaction = new RawTransaction(TransactionVersion.V2)

await node.wallet.addSpendsForAsset(
rawTransaction,
accountA,
Asset.nativeId(),
const note = new NativeNote(
accountA.publicAddress,
BigInt(ORE_TO_IRON * 10),
0n,
new BufferSet(),
0,
Buffer.alloc(0),
Asset.nativeId(),
accountA.publicAddress,
)

rawTransaction.outputs.push({ note: new Note(note.serialize()) })

await node.wallet.fund(rawTransaction, {
account: accountA,
confirmations: 0,
})

// if this fails, it means that the notes were not sorted in descending order
// multiple smaller notes were used to fund the transaction
expect(rawTransaction.spends).toHaveLength(1)
})

it('should throw error if transaction exceeds maximum size', async () => {
const { node } = nodeTest

const account = await useAccountFixture(node.wallet, 'a')

const block1 = await useMinerBlockFixture(node.chain, undefined, account, node.wallet)
await expect(node.chain).toAddBlock(block1)
await node.wallet.scan()

// Mock verifier to only allow transactions of size 0
jest.spyOn(Verifier, 'getMaxTransactionBytes').mockImplementationOnce((_) => 0)

const rawTransaction = new RawTransaction(TransactionVersion.V2)
const note = new NativeNote(
account.publicAddress,
1n,
Buffer.alloc(0),
Asset.nativeId(),
account.publicAddress,
)

rawTransaction.outputs.push({ note: new Note(note.serialize()) })

const promise = node.wallet.fund(rawTransaction, {
account: account,
confirmations: 0,
})

await expect(promise).rejects.toThrow(MaxTransactionSizeError)
})
})

describe('getBalances', () => {
Expand Down Expand Up @@ -1471,7 +1506,7 @@ describe('Wallet', () => {
await node.wallet.scan()

// Mock verifier to only allow transactions of size 0
jest.spyOn(Verifier, 'getMaxTransactionBytes').mockImplementation((_) => 0)
jest.spyOn(Verifier, 'getMaxTransactionBytes').mockImplementationOnce((_) => 0)

const promise = nodeTest.wallet.createTransaction({
account: account,
Expand Down
84 changes: 38 additions & 46 deletions ironfish/src/wallet/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { MintDescription } from '../primitives/mintDescription'
import { Note } from '../primitives/note'
import { NoteEncrypted } from '../primitives/noteEncrypted'
import { MintData, RawTransaction } from '../primitives/rawTransaction'
import { SPEND_SERIALIZED_SIZE_IN_BYTE } from '../primitives/spend'
import { Transaction } from '../primitives/transaction'
import { GetBlockRequest, GetBlockResponse, RpcClient } from '../rpc'
import { IDatabaseTransaction } from '../storage/database/transaction'
Expand Down Expand Up @@ -939,9 +940,7 @@ export class Wallet {
this.consensus.parameters.maxBlockSizeBytes,
)
if (raw.postedSize() > maxTransactionSize) {
throw new MaxTransactionSizeError(
`Proposed transaction is larger than maximum transaction size of ${maxTransactionSize} bytes`,
)
throw new MaxTransactionSizeError(maxTransactionSize)
}

return raw
Expand Down Expand Up @@ -1009,6 +1008,11 @@ export class Wallet {
confirmations: number
},
): Promise<void> {
let postedSize = raw.postedSize()
const maxTransactionSize = Verifier.getMaxTransactionBytes(
this.consensus.parameters.maxBlockSizeBytes,
)

const needed = this.buildAmountsNeeded(raw, { fee: raw.fee })
const spent = new BufferMap<bigint>()
const notesSpent = new BufferMap<BufferSet>()
Expand All @@ -1034,28 +1038,47 @@ export class Wallet {
notesSpent.set(assetId, assetNotesSpent)

raw.spends.push({ note: decryptedNote.note, witness })

postedSize += SPEND_SERIALIZED_SIZE_IN_BYTE
if (postedSize > maxTransactionSize) {
throw new MaxTransactionSizeError(maxTransactionSize)
}
}

for (const [assetId, assetAmountNeeded] of needed.entries()) {
const assetAmountSpent = spent.get(assetId) ?? 0n
let assetAmountSpent = spent.get(assetId) ?? 0n
const assetNotesSpent = notesSpent.get(assetId) ?? new BufferSet()

if (assetAmountSpent >= assetAmountNeeded) {
continue
}

const amountSpent = await this.addSpendsForAsset(
raw,
options.account,
assetId,
assetAmountNeeded,
assetAmountSpent,
assetNotesSpent,
options.confirmations,
)
for await (const unspentNote of options.account.getUnspentNotes(assetId, {
reverse: true,
confirmations: options.confirmations,
})) {
if (assetNotesSpent.has(unspentNote.note.hash())) {
continue
}

const witness = await this.getNoteWitness(unspentNote, options.confirmations)

assetAmountSpent += unspentNote.note.value()

raw.spends.push({ note: unspentNote.note, witness })

if (amountSpent < assetAmountNeeded) {
throw new NotEnoughFundsError(assetId, amountSpent, assetAmountNeeded)
postedSize += SPEND_SERIALIZED_SIZE_IN_BYTE
if (postedSize > maxTransactionSize) {
throw new MaxTransactionSizeError(maxTransactionSize)
}

if (assetAmountSpent >= assetAmountNeeded) {
break
}
}

if (assetAmountSpent < assetAmountNeeded) {
throw new NotEnoughFundsError(assetId, assetAmountSpent, assetAmountNeeded)
}
}
}
Expand Down Expand Up @@ -1122,37 +1145,6 @@ export class Wallet {
return amountsNeeded
}

async addSpendsForAsset(
raw: RawTransaction,
sender: Account,
assetId: Buffer,
amountNeeded: bigint,
amountSpent: bigint,
notesSpent: BufferSet,
confirmations: number,
): Promise<bigint> {
for await (const unspentNote of sender.getUnspentNotes(assetId, {
reverse: true,
confirmations,
})) {
if (notesSpent.has(unspentNote.note.hash())) {
continue
}

const witness = await this.getNoteWitness(unspentNote, confirmations)

amountSpent += unspentNote.note.value()

raw.spends.push({ note: unspentNote.note, witness })

if (amountSpent >= amountNeeded) {
break
}
}

return amountSpent
}

async broadcastTransaction(
transaction: Transaction,
): Promise<{ accepted: boolean; broadcasted: boolean }> {
Expand Down

0 comments on commit 5ecdcef

Please sign in to comment.