diff --git a/src/crypto/secp256k1.ts b/src/crypto/secp256k1.ts index 77a13f52d..891e17fcf 100644 --- a/src/crypto/secp256k1.ts +++ b/src/crypto/secp256k1.ts @@ -4,6 +4,7 @@ import * as secp from '@noble/secp256k1'; import { Address } from 'micro-eth-signer'; import { concatBytes, hexToBuffer } from '../utils/buffer'; +/** Number of bytes per signature */ export const SIGNATURE_LENGTH = 65; export function randomPrivateKey() { diff --git a/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.test.ts b/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.test.ts index 059873e22..2f233833d 100644 --- a/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.test.ts +++ b/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.test.ts @@ -7,6 +7,12 @@ import { getInitialReducerState, getSpendHelper, } from './fixtures/reducers'; +import { + BigIntPr, + Id, + TransferOutput, + TransferableOutput, +} from '../../../../serializable'; describe('handleFeeAndChange', () => { test('throws an error if excessAVAX is less than the required fee', () => { @@ -25,15 +31,11 @@ describe('handleFeeAndChange', () => { const state = getInitialReducerState({ excessAVAX: 4n }); const spendHelper = getSpendHelper(); const addChangeOutputSpy = jest.spyOn(spendHelper, 'addChangeOutput'); - const calculateFeeWithTemporaryOutputComplexitySpy = jest.spyOn( - spendHelper, - 'calculateFeeWithTemporaryOutputComplexity', - ); + const calculateFeeSpy = jest.spyOn(spendHelper, 'calculateFee'); - expect(handleFeeAndChange(state, getSpendHelper(), testContext)).toEqual( - state, - ); - expect(calculateFeeWithTemporaryOutputComplexitySpy).not.toHaveBeenCalled(); + expect(handleFeeAndChange(state, spendHelper, testContext)).toEqual(state); + expect(calculateFeeSpy).toHaveBeenCalledTimes(1); + expect(calculateFeeSpy).toHaveBeenCalledWith(); expect(addChangeOutputSpy).not.toHaveBeenCalled(); }); @@ -45,17 +47,18 @@ describe('handleFeeAndChange', () => { const spendHelper = getSpendHelper(); const addChangeOutputSpy = jest.spyOn(spendHelper, 'addChangeOutput'); - const calculateFeeWithTemporaryOutputComplexitySpy = jest.spyOn( - spendHelper, - 'calculateFeeWithTemporaryOutputComplexity', - ); + const calculateFeeSpy = jest.spyOn(spendHelper, 'calculateFee'); expect(handleFeeAndChange(state, spendHelper, testContext)).toEqual({ ...state, excessAVAX, }); - expect(calculateFeeWithTemporaryOutputComplexitySpy).toHaveBeenCalledTimes( - 1, + expect(calculateFeeSpy).toHaveBeenCalledTimes(2); + expect(calculateFeeSpy).toHaveBeenCalledWith( + new TransferableOutput( + Id.fromString(testContext.avaxAssetID), + new TransferOutput(new BigIntPr(0n), CHANGE_OWNERS), + ), ); expect(addChangeOutputSpy).toHaveBeenCalledTimes(1); diff --git a/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.ts b/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.ts index 644b31b17..94f7f9a08 100644 --- a/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.ts +++ b/src/vms/pvm/etna-builder/spend-reducers/handleFeeAndChange.ts @@ -72,17 +72,16 @@ export const handleFeeAndChange: SpendReducerFunction = ( } else { // Calculate the fee with a temporary output complexity // as if we added the change output. - const requiredFeeWithChangeOutput = - spendHelper.calculateFeeWithTemporaryOutputComplexity( - new TransferableOutput( - Id.fromString(context.avaxAssetID), - new TransferOutput(new BigIntPr(0n), changeOwners), - ), - ); + const requiredFeeWithChangeOutput = spendHelper.calculateFee( + new TransferableOutput( + Id.fromString(context.avaxAssetID), + new TransferOutput(new BigIntPr(0n), changeOwners), + ), + ); // If the excess AVAX is greater than the new fee, add a change output. // Otherwise, ignore and burn the excess because it can't be returned - // (ie we can't pay the fee to return the excess). + // (ie there is no point in adding a change output if you can't afford to add it). if (state.excessAVAX > requiredFeeWithChangeOutput) { spendHelper.addChangeOutput( new TransferableOutput( diff --git a/src/vms/pvm/etna-builder/spend-reducers/useUnlockedUTXOs.ts b/src/vms/pvm/etna-builder/spend-reducers/useUnlockedUTXOs.ts index 8b9c4be37..1883a83ea 100644 --- a/src/vms/pvm/etna-builder/spend-reducers/useUnlockedUTXOs.ts +++ b/src/vms/pvm/etna-builder/spend-reducers/useUnlockedUTXOs.ts @@ -63,19 +63,31 @@ export const useUnlockedUTXOs: SpendReducerFunction = ( ); // 3. Split verified usable UTXOs into AVAX assetId UTXOs and other assetId UTXOs. - const [otherVerifiedUsableUTXOs, avaxVerifiedUsableUTXOs] = - verifiedUsableUTXOs.reduce( - (result, { sigData, data: utxo }) => { - if (utxo.assetId.value() === context.avaxAssetID) { - return [result[0], [...result[1], { sigData, data: utxo }]]; + const { otherVerifiedUsableUTXOs, avaxVerifiedUsableUTXOs } = + verifiedUsableUTXOs.reduce<{ + avaxVerifiedUsableUTXOs: typeof verifiedUsableUTXOs; + otherVerifiedUsableUTXOs: typeof verifiedUsableUTXOs; + }>( + (result, verifiedUsableUTXO) => { + if (verifiedUsableUTXO.data.assetId.value() === context.avaxAssetID) { + return { + ...result, + avaxVerifiedUsableUTXOs: [ + ...result.avaxVerifiedUsableUTXOs, + verifiedUsableUTXO, + ], + }; } - return [[...result[0], { sigData, data: utxo }], result[1]]; + return { + ...result, + otherVerifiedUsableUTXOs: [ + ...result.otherVerifiedUsableUTXOs, + verifiedUsableUTXO, + ], + }; }, - [[], []] as [ - other: typeof verifiedUsableUTXOs, - avax: typeof verifiedUsableUTXOs, - ], + { otherVerifiedUsableUTXOs: [], avaxVerifiedUsableUTXOs: [] }, ); // 4. Handle all the non-AVAX asset UTXOs first. diff --git a/src/vms/pvm/etna-builder/spend.ts b/src/vms/pvm/etna-builder/spend.ts index a68b39c0e..3471e729e 100644 --- a/src/vms/pvm/etna-builder/spend.ts +++ b/src/vms/pvm/etna-builder/spend.ts @@ -58,7 +58,7 @@ export type SpendProps = Readonly<{ */ ownerOverride?: OutputOwners | null; /** - * Whether to consolidate outputs. + * Whether to consolidate change and stake outputs. * * @default false */ @@ -100,7 +100,7 @@ export type SpendProps = Readonly<{ */ export const spend = ( { - excessAVAX: _excessAVAX = 0n, + excessAVAX = 0n, fromAddresses, initialComplexity, ownerOverride, @@ -116,7 +116,6 @@ export const spend = ( try { const changeOwners = ownerOverride || OutputOwners.fromNative(spendOptions.changeAddresses); - const excessAVAX: bigint = _excessAVAX; const spendHelper = new SpendHelper({ changeOutputs: [], @@ -143,9 +142,6 @@ export const spend = ( const spendReducerFunctions: readonly SpendReducerFunction[] = [ ...spendReducers, - // useSpendableLockedUTXOs, - // TODO: Should we just default include this? Used on every builder. - // useUnlockedUTXOs, verifyAssetsConsumed, handleFeeAndChange, // Consolidation and sorting happens in the SpendHelper. diff --git a/src/vms/pvm/etna-builder/spendHelper.test.ts b/src/vms/pvm/etna-builder/spendHelper.test.ts index 211bd517c..50c42c982 100644 --- a/src/vms/pvm/etna-builder/spendHelper.test.ts +++ b/src/vms/pvm/etna-builder/spendHelper.test.ts @@ -61,10 +61,6 @@ describe('src/vms/pvm/etna-builder/spendHelper', () => { stakeOutputs: [], }); - spendHelper.addOutputComplexity(transferableOutput()); - - expect(spendHelper.calculateFee()).toBe(339n); - const inputUtxo = utxo(); const inputTransferableInput = transferableInput(); @@ -72,7 +68,7 @@ describe('src/vms/pvm/etna-builder/spendHelper', () => { expect(spendHelper.getInputsOutputs()).toEqual({ changeOutputs: [], - fee: 1251n, + fee: 942n, inputs: [inputTransferableInput], inputUTXOs: [inputUtxo], stakeOutputs: [], @@ -84,7 +80,7 @@ describe('src/vms/pvm/etna-builder/spendHelper', () => { expect(spendHelper.getInputsOutputs()).toEqual({ changeOutputs: [changeOutput], - fee: 1560n, + fee: 1251n, inputs: [inputTransferableInput], inputUTXOs: [inputUtxo], stakeOutputs: [], @@ -96,7 +92,7 @@ describe('src/vms/pvm/etna-builder/spendHelper', () => { expect(spendHelper.getInputsOutputs()).toEqual({ changeOutputs: [changeOutput], - fee: 1869n, + fee: 1560n, inputs: [inputTransferableInput], inputUTXOs: [inputUtxo], stakeOutputs: [stakeOutput], @@ -397,9 +393,9 @@ describe('src/vms/pvm/etna-builder/spendHelper', () => { const temporaryOutput = transferableOutput(); - expect( - spendHelper.calculateFeeWithTemporaryOutputComplexity(temporaryOutput), - ).toBeGreaterThan(originalFee); + expect(spendHelper.calculateFee(temporaryOutput)).toBeGreaterThan( + originalFee, + ); expect(spendHelper.calculateFee()).toBe(originalFee); }); diff --git a/src/vms/pvm/etna-builder/spendHelper.ts b/src/vms/pvm/etna-builder/spendHelper.ts index d7f4bcabe..54699f311 100644 --- a/src/vms/pvm/etna-builder/spendHelper.ts +++ b/src/vms/pvm/etna-builder/spendHelper.ts @@ -41,8 +41,6 @@ export class SpendHelper { private changeOutputs: readonly TransferableOutput[]; private inputs: readonly TransferableInput[]; - private inputComplexity: Dimensions = createEmptyDimensions(); - private outputComplexity: Dimensions = createEmptyDimensions(); private stakeOutputs: readonly TransferableOutput[]; private inputUTXOs: readonly Utxo[] = []; @@ -78,13 +76,6 @@ export class SpendHelper { * @returns {SpendHelper} The current instance of SpendHelper for chaining. */ addInput(utxo: Utxo, transferableInput: TransferableInput): SpendHelper { - const newInputComplexity = getInputComplexity([transferableInput]); - - this.inputComplexity = addDimensions( - this.inputComplexity, - newInputComplexity, - ); - this.inputs = [...this.inputs, transferableInput]; this.inputUTXOs = [...this.inputUTXOs, utxo]; @@ -118,29 +109,19 @@ export class SpendHelper { } /** - * Adds a transferable output to the SpendHelper. - * - * @param {TransferableOutput} transferableOutput - The transferable output to be added. - * @returns {SpendHelper} The current instance of SpendHelper for chaining. + * When computing the complexity/fee of a transaction that needs change but doesn't yet have + * a corresponding change output, `additionalComplexity` may be used to calculate the complexity + * and therefore the fee as if the change output was already added. */ - addOutputComplexity(transferableOutput: TransferableOutput): SpendHelper { - const newOutputComplexity = getOutputComplexity([transferableOutput]); - - this.outputComplexity = addDimensions( - this.outputComplexity, - newOutputComplexity, - ); - - return this; - } - - private getComplexity(): Dimensions { + private getComplexity( + additionalComplexity: Dimensions = createEmptyDimensions(), + ): Dimensions { return addDimensions( this.initialComplexity, getInputComplexity(this.inputs), getOutputComplexity(this.changeOutputs), getOutputComplexity(this.stakeOutputs), - this.outputComplexity, + additionalComplexity, ); } @@ -231,30 +212,24 @@ export class SpendHelper { /** * Calculates the fee for the SpendHelper based on its complexity and gas price. + * Provide an empty change output as a parameter to calculate the fee as if the change output was already added. * + * @param {TransferableOutput} additionalOutput - The change output that has not yet been added to the SpendHelper. * @returns {bigint} The fee for the SpendHelper. */ - calculateFee(): bigint { + calculateFee(additionalOutput?: TransferableOutput): bigint { this.consolidateOutputs(); - const gas = dimensionsToGas(this.getComplexity(), this.weights); + const gas = dimensionsToGas( + this.getComplexity( + additionalOutput ? getOutputComplexity([additionalOutput]) : undefined, + ), + this.weights, + ); return gas * this.gasPrice; } - calculateFeeWithTemporaryOutputComplexity( - transferableOutput: TransferableOutput, - ): bigint { - const oldOutputComplexity = this.outputComplexity; - this.addOutputComplexity(transferableOutput); - - const fee = this.calculateFee(); - - this.outputComplexity = oldOutputComplexity; - - return fee; - } - /** * Determines if a change output with a matching asset ID and output owners exists. *