From dcd67f1f0fe87b6c3ad96b698a88e4d727b2937a Mon Sep 17 00:00:00 2001 From: Angel Castillo Date: Tue, 30 May 2023 20:21:53 +0800 Subject: [PATCH] fixup! feat(input-selection): added new greedy input selector --- .../GreedySelection/GreedyInputSelector.ts | 8 +- packages/input-selection/src/util.ts | 36 +- packages/input-selection/test/util.test.ts | 347 +++++++++++++++++- 3 files changed, 360 insertions(+), 31 deletions(-) diff --git a/packages/input-selection/src/GreedySelection/GreedyInputSelector.ts b/packages/input-selection/src/GreedySelection/GreedyInputSelector.ts index b36e84392a8..87ef9cbd272 100644 --- a/packages/input-selection/src/GreedySelection/GreedyInputSelector.ts +++ b/packages/input-selection/src/GreedySelection/GreedyInputSelector.ts @@ -3,13 +3,13 @@ import { Cardano } from '@cardano-sdk/core'; import { InputSelectionError, InputSelectionFailure } from '../InputSelectionError'; import { InputSelectionParameters, InputSelector, SelectionConstraints, SelectionResult } from '../types'; import { - getAssetAddition, - getAssetDifference, + addTokenMaps, getAssetQuantities, getCoinQuantity, hasNegativeAssetValue, sortByCoins, splitChange, + subtractTokenMaps, toValues } from '../util'; @@ -113,12 +113,12 @@ export class GreedyInputSelector implements InputSelector { const implicitAssetInput = implicitValue?.mint || new Map(); const totalLovelaceInput = totalLovelaceInUtxoSet + implicitCoinInput; const totalLovelaceOutput = totalLovelaceInOutputSet + implicitCoinOutput; - const totalAssetsInput = getAssetAddition(totalAssetsInUtxoSet, implicitAssetInput); + const totalAssetsInput = addTokenMaps(totalAssetsInUtxoSet, implicitAssetInput); // Calculate the difference between the given outputs and the UTXO set. This will let us know // how much we need to return as change. const changeLovelace = totalLovelaceOutput - totalLovelaceInput; - const changeAssets = getAssetDifference(totalAssetsInOutputSet, totalAssetsInput); + const changeAssets = subtractTokenMaps(totalAssetsInOutputSet, totalAssetsInput); // If the wallet tries to spend more than we can cover with the current UTXO set + implicit value throw. if (changeLovelace <= 0n || hasNegativeAssetValue(changeAssets)) diff --git a/packages/input-selection/src/util.ts b/packages/input-selection/src/util.ts index c0ee2f6ceb4..8969cf47805 100644 --- a/packages/input-selection/src/util.ts +++ b/packages/input-selection/src/util.ts @@ -6,6 +6,8 @@ import { ComputeMinimumCoinQuantity, ImplicitValue, TokenBundleSizeExceedsLimit import { InputSelectionError, InputSelectionFailure } from './InputSelectionError'; import uniq from 'lodash/uniq'; +const PERCENTAGE_TOLERANCE = 0.05; + export const stubMaxSizeAddress = Cardano.PaymentAddress( 'addr_test1qqydn46r6mhge0kfpqmt36m6q43knzsd9ga32n96m89px3nuzcjqw982pcftgx53fu5527z2cj2tkx2h8ux2vxsg475qypp3m9' ); @@ -193,7 +195,7 @@ export const sortByCoins = (lhs: Cardano.TxOut, rhs: Cardano.TxOut) => { * @param rhs the right-hand side of the subtraction operation. * @returns The difference between both TokenMaps. */ -export const getAssetDifference = ( +export const subtractTokenMaps = ( lhs: Cardano.TokenMap | undefined, rhs: Cardano.TokenMap | undefined ): Cardano.TokenMap | undefined => { @@ -253,7 +255,7 @@ export const getAssetDifference = ( * @param rhs the right-hand side of the addition operation. * @returns The addition between both TokenMaps. */ -export const getAssetAddition = ( +export const addTokenMaps = ( lhs: Cardano.TokenMap | undefined, rhs: Cardano.TokenMap | undefined ): Cardano.TokenMap | undefined => { @@ -356,16 +358,15 @@ const distributeAssets = ( throw new InputSelectionError(InputSelectionFailure.UtxoBalanceInsufficient); } - if (!output.value.assets) { + if (!output.value.assets || output.value.assets.size === 0) { // If this output failed and doesn't contain any assets, it means there is not enough coins to cover - // the min ADA coin pero UTXO. We will throw for now, but it may be possible to fix this by moving some excess - // lovelace from one of the other outputs (ignoring the given distribution). + // the min ADA coin per UTXO even after moving all the assets to the other outputs. throw new InputSelectionError(InputSelectionFailure.UtxoBalanceInsufficient); } const splicedAsset = new Map([...output.value.assets!.entries()].splice(0, 1)); - const currentOutputNewAssets = getAssetDifference(output.value.assets, splicedAsset); - const nextOutputNewAssets = getAssetAddition(adjustedOutputs[i + 1].value.assets, splicedAsset); + const currentOutputNewAssets = subtractTokenMaps(output.value.assets, splicedAsset); + const nextOutputNewAssets = addTokenMaps(adjustedOutputs[i + 1].value.assets, splicedAsset); output.value.assets = currentOutputNewAssets; adjustedOutputs[i + 1].value.assets = nextOutputNewAssets; @@ -405,6 +406,12 @@ export const splitChange = async ( tokenBundleSizeExceedsLimit: TokenBundleSizeExceedsLimit ): Promise> => { const changeAddresses = await getChangeAddresses(); + const totalPercentage = [...changeAddresses.values()].reduce((sum, current) => sum + current, 0); + + // We are going to enforce that the given % mostly add up to 100% (to account for division errors) + if (Math.abs(1 - totalPercentage) > PERCENTAGE_TOLERANCE) + throw new InputSelectionError(InputSelectionFailure.UtxoBalanceInsufficient); // TODO: We need a new error for this types of failures + const changeOutputs: Array = [...changeAddresses.entries()].map((val) => ({ address: val[0], value: { coins: 0n } @@ -424,20 +431,9 @@ export const splitChange = async ( runningTotal > totalChangeLovelace ? coinAllocation - (runningTotal - totalChangeLovelace) : coinAllocation; } - const totalAllocated = changeOutputs.reduce( - (prev, current) => { - current.value.coins += prev.value.coins; - return current; - }, - { - address: '', - value: { coins: 0n } - } - ).value.coins; - - if (totalAllocated < totalChangeLovelace) { + if (runningTotal < totalChangeLovelace) { // This may be because the given proportions don't add up to 100%. We will add the missing balance to the last output. - const missingAllocation = totalChangeLovelace - totalAllocated; + const missingAllocation = totalChangeLovelace - runningTotal; changeOutputs[changeOutputs.length - 1].value.coins += missingAllocation; } diff --git a/packages/input-selection/test/util.test.ts b/packages/input-selection/test/util.test.ts index 661563a1ce2..1c5f6f36a07 100644 --- a/packages/input-selection/test/util.test.ts +++ b/packages/input-selection/test/util.test.ts @@ -1,7 +1,17 @@ import { Cardano } from '@cardano-sdk/core'; -import { getAssetDifference, hasNegativeAssetValue, isValidValue, sortByCoins, stubMaxSizeAddress } from '../src/util'; +import { InputSelectionError, InputSelectionFailure } from '../src'; +import { + addTokenMaps, + hasNegativeAssetValue, + isValidValue, + sortByCoins, + splitChange, + stubMaxSizeAddress, + subtractTokenMaps +} from '../src/util'; const getAsAssetId = (x: string): Cardano.AssetId => x as unknown as Cardano.AssetId; +const getAsPaymentAddress = (x: string): Cardano.PaymentAddress => x as unknown as Cardano.PaymentAddress; const getAsTokenMap = (elements: Iterable<[Cardano.AssetId, bigint]>) => new Map(elements); describe('sortByCoins', () => { @@ -42,12 +52,12 @@ describe('sortByCoins', () => { }); }); -describe('getAssetDifference', () => { +describe('subtractTokenMaps', () => { it('returns an empty token map when the two arguments are empty maps', async () => { const lhs = new Map(); const rhs = new Map(); - const result = getAssetDifference(lhs, rhs); + const result = subtractTokenMaps(lhs, rhs); expect(result).toEqual(new Map()); }); @@ -72,7 +82,7 @@ describe('getAssetDifference', () => { rhs.set(getAsAssetId('6'), 500n); lhs.set(getAsAssetId('6'), 1000n); - const result = getAssetDifference(lhs, rhs); + const result = subtractTokenMaps(lhs, rhs); expect(result).toEqual( getAsTokenMap([ @@ -99,7 +109,7 @@ describe('getAssetDifference', () => { rhs.set(getAsAssetId('6'), 500n); lhs.set(getAsAssetId('6'), 1000n); - const result = getAssetDifference(lhs, rhs); + const result = subtractTokenMaps(lhs, rhs); expect(result).toEqual( getAsTokenMap([ @@ -112,7 +122,7 @@ describe('getAssetDifference', () => { ); }); - it('returns returns the correct result when there are values missing in both sides of the subtraction', async () => { + it('returns the correct result when there are values missing in both sides of the subtraction', async () => { const lhs = new Map(); const rhs = new Map(); @@ -122,7 +132,7 @@ describe('getAssetDifference', () => { lhs.set(getAsAssetId('3'), 1500n); rhs.set(getAsAssetId('4'), 2500n); - const result = getAssetDifference(lhs, rhs); + const result = subtractTokenMaps(lhs, rhs); expect(result).toEqual( getAsTokenMap([ @@ -134,6 +144,109 @@ describe('getAssetDifference', () => { ]) ); }); + + it('returns the lhs if rhs is undefined', async () => { + const lhs = new Map(); + const rhs = undefined; + + lhs.set(getAsAssetId('0'), 100n); + lhs.set(getAsAssetId('1'), 23n); + lhs.set(getAsAssetId('2'), 1n); + lhs.set(getAsAssetId('3'), 1500n); + + const result = subtractTokenMaps(lhs, rhs); + + expect(result).toEqual(lhs); + }); + + it('returns the rhs with inverted signs if lhs is undefined', async () => { + const rhs = new Map(); + const lhs = undefined; + + rhs.set(getAsAssetId('0'), 100n); + rhs.set(getAsAssetId('1'), -23n); + rhs.set(getAsAssetId('2'), 1n); + rhs.set(getAsAssetId('3'), 1500n); + + const result = subtractTokenMaps(lhs, rhs); + + expect(result).toEqual( + getAsTokenMap([ + [getAsAssetId('0'), -100n], + [getAsAssetId('1'), 23n], + [getAsAssetId('2'), -1n], + [getAsAssetId('3'), -1500n] + ]) + ); + }); +}); + +describe('addTokenMaps', () => { + it('returns an empty token map when the two arguments are empty maps', async () => { + const lhs = new Map(); + const rhs = new Map(); + + const result = addTokenMaps(lhs, rhs); + + expect(result).toEqual(new Map()); + }); + + it('returns the aggregate of both maps, if the asset is in both maps adds them together', async () => { + const lhs = new Map(); + const rhs = new Map(); + + rhs.set(getAsAssetId('0'), 100n); + rhs.set(getAsAssetId('1'), 23n); + rhs.set(getAsAssetId('2'), 1n); + + rhs.set(getAsAssetId('3'), 1000n); + // Should be present as 1500n + rhs.set(getAsAssetId('4'), 1000n); + lhs.set(getAsAssetId('4'), 500n); + // Should be present as 500n + lhs.set(getAsAssetId('5'), 500n); + + const result = addTokenMaps(lhs, rhs); + + expect(result).toEqual( + getAsTokenMap([ + [getAsAssetId('0'), 100n], + [getAsAssetId('1'), 23n], + [getAsAssetId('2'), 1n], + [getAsAssetId('3'), 1000n], + [getAsAssetId('4'), 1500n], + [getAsAssetId('5'), 500n] + ]) + ); + }); + + it('returns the lhs if rhs is undefined', async () => { + const lhs = new Map(); + const rhs = undefined; + + lhs.set(getAsAssetId('0'), 100n); + lhs.set(getAsAssetId('1'), 23n); + lhs.set(getAsAssetId('2'), 1n); + lhs.set(getAsAssetId('3'), 1500n); + + const result = addTokenMaps(lhs, rhs); + + expect(result).toEqual(lhs); + }); + + it('returns the rhs if lhs is undefined', async () => { + const rhs = new Map(); + const lhs = undefined; + + rhs.set(getAsAssetId('0'), 100n); + rhs.set(getAsAssetId('1'), -23n); + rhs.set(getAsAssetId('2'), 1n); + rhs.set(getAsAssetId('3'), 1500n); + + const result = addTokenMaps(lhs, rhs); + + expect(result).toEqual(rhs); + }); }); describe('hasNegativeAssetValue', () => { @@ -206,3 +319,223 @@ describe('isValidValue', () => { ).toBeFalsy(); }); }); + +describe('splitChange', () => { + it('correctly split pure lovelace change', async () => { + const changeAddressWithDistribution = new Map([ + [getAsPaymentAddress('A'), 0.34], + [getAsPaymentAddress('B'), 0.33], + [getAsPaymentAddress('C'), 0.33] + ]); + + const change = await splitChange( + async () => changeAddressWithDistribution, + 100n, + undefined, + () => 10n, + () => false + ); + + expect(change.length).toEqual(3); + expect(change[0].address).toEqual(getAsPaymentAddress('A')); + expect(change[0].value.coins).toEqual(34n); + expect(change[1].address).toEqual(getAsPaymentAddress('B')); + expect(change[1].value.coins).toEqual(33n); + expect(change[2].address).toEqual(getAsPaymentAddress('C')); + expect(change[2].value.coins).toEqual(33n); + }); + + it('pushes the error to the last token', async () => { + const changeAddressWithDistribution = new Map([ + [getAsPaymentAddress('A'), 1 / 3], + [getAsPaymentAddress('B'), 1 / 3], + [getAsPaymentAddress('C'), 1 / 3] + ]); + + const change = await splitChange( + async () => changeAddressWithDistribution, + 100n, + undefined, + () => 10n, + () => false + ); + + expect(change.length).toEqual(3); + expect(change[0].address).toEqual(getAsPaymentAddress('A')); + expect(change[0].value.coins).toEqual(34n); + expect(change[1].address).toEqual(getAsPaymentAddress('B')); + expect(change[1].value.coins).toEqual(34n); + expect(change[2].address).toEqual(getAsPaymentAddress('C')); + expect(change[2].value.coins).toEqual(32n); // Rounding error is pushed to the last entry + }); + + it('allocates native assets on the first output', async () => { + const changeAddressWithDistribution = new Map([ + [getAsPaymentAddress('A'), 1 / 3], + [getAsPaymentAddress('B'), 1 / 3], + [getAsPaymentAddress('C'), 1 / 3] + ]); + + const assets = getAsTokenMap([ + [getAsAssetId('0'), 100n], + [getAsAssetId('1'), 23n], + [getAsAssetId('2'), 1n], + [getAsAssetId('3'), 1000n], + [getAsAssetId('4'), 1500n], + [getAsAssetId('5'), 500n] + ]); + + const change = await splitChange( + async () => changeAddressWithDistribution, + 10_000_000n, // 10 ADA in change + assets, + () => 10n, + () => false + ); + + expect(change.length).toEqual(3); + expect(change[0].address).toEqual(getAsPaymentAddress('A')); + expect(change[0].value.coins).toEqual(3_333_334n); + expect(change[0].value.assets).toEqual(assets); + expect(change[1].address).toEqual(getAsPaymentAddress('B')); + expect(change[1].value.coins).toEqual(3_333_334n); + expect(change[1].value.assets).toBeUndefined(); + expect(change[2].address).toEqual(getAsPaymentAddress('C')); + expect(change[2].value.coins).toEqual(3_333_332n); + expect(change[2].value.assets).toBeUndefined(); + }); + + it('spills over native assets to other change outputs if they dont fit', async () => { + const changeAddressWithDistribution = new Map([ + [getAsPaymentAddress('A'), 1 / 3], + [getAsPaymentAddress('B'), 1 / 3], + [getAsPaymentAddress('C'), 1 / 3] + ]); + + const assets = getAsTokenMap([ + [getAsAssetId('0'), 100n], + [getAsAssetId('1'), 23n], + [getAsAssetId('2'), 1n], + [getAsAssetId('3'), 1000n], + [getAsAssetId('4'), 1500n], + [getAsAssetId('5'), 500n] + ]); + + const change = await splitChange( + async () => changeAddressWithDistribution, + 10_000_000n, // 10 ADA in change + assets, + () => 10n, + (tokenBundle: Cardano.TokenMap | undefined) => tokenBundle!.size > 4 // Impose an artificial limit of four asset class per output. + ); + + expect(change.length).toEqual(3); + expect(change[0].address).toEqual(getAsPaymentAddress('A')); + expect(change[0].value.coins).toEqual(3_333_334n); + expect(change[0].value.assets).toEqual( + getAsTokenMap([ + [getAsAssetId('2'), 1n], + [getAsAssetId('3'), 1000n], + [getAsAssetId('4'), 1500n], + [getAsAssetId('5'), 500n] + ]) + ); + expect(change[1].address).toEqual(getAsPaymentAddress('B')); + expect(change[1].value.coins).toEqual(3_333_334n); + expect(change[1].value.assets).toEqual( + getAsTokenMap([ + [getAsAssetId('0'), 100n], + [getAsAssetId('1'), 23n] + ]) + ); + expect(change[2].address).toEqual(getAsPaymentAddress('C')); + expect(change[2].value.coins).toEqual(3_333_332n); + expect(change[2].value.assets).toBeUndefined(); + }); + + it('throws InputSelectionError with UtxoBalanceInsufficient if it cant fit the native assets in the change outputs', async () => { + const changeAddressWithDistribution = new Map([ + [getAsPaymentAddress('A'), 1 / 3], + [getAsPaymentAddress('B'), 1 / 3], + [getAsPaymentAddress('C'), 1 / 3] + ]); + + const assets = getAsTokenMap([ + [getAsAssetId('0'), 100n], + [getAsAssetId('1'), 23n], + [getAsAssetId('2'), 1n], + [getAsAssetId('3'), 1000n], + [getAsAssetId('4'), 1500n], + [getAsAssetId('5'), 500n] + ]); + + await expect( + splitChange( + async () => changeAddressWithDistribution, + 10_000_000n, // 10 ADA in change + assets, + () => 10n, + (tokenBundle: Cardano.TokenMap | undefined) => tokenBundle!.size > 1 // Impose an artificial limit of one asset class per output. + ) + ).rejects.toThrow(new InputSelectionError(InputSelectionFailure.UtxoBalanceInsufficient)); + }); + + it('throws InputSelectionError with UtxoBalanceInsufficient if it doesnt have enough lovelace to fulfill minimum coin quantity per output', async () => { + const changeAddressWithDistribution = new Map([ + [getAsPaymentAddress('A'), 1 / 3], + [getAsPaymentAddress('B'), 1 / 3], + [getAsPaymentAddress('C'), 1 / 3] + ]); + + const assets = getAsTokenMap([ + [getAsAssetId('0'), 100n], + [getAsAssetId('1'), 23n], + [getAsAssetId('2'), 1n], + [getAsAssetId('3'), 1000n], + [getAsAssetId('4'), 1500n], + [getAsAssetId('5'), 500n] + ]); + + await expect( + splitChange( + async () => changeAddressWithDistribution, + 10n, + assets, + () => 10n, + () => false + ) + ).rejects.toThrow(new InputSelectionError(InputSelectionFailure.UtxoBalanceInsufficient)); + }); + + it('throws InputSelectionError with UtxoBalanceInsufficient if the given % dont add up to 1.0 (within a tolerance)', async () => { + await expect( + splitChange( + async () => + new Map([ + [getAsPaymentAddress('A'), 0.3], + [getAsPaymentAddress('B'), 0.3], + [getAsPaymentAddress('C'), 0.2] + ]), + 10n, + undefined, + () => 10n, + () => false + ) + ).rejects.toThrow(new InputSelectionError(InputSelectionFailure.UtxoBalanceInsufficient)); + + await expect( + splitChange( + async () => + new Map([ + [getAsPaymentAddress('A'), 0.3], + [getAsPaymentAddress('B'), 0.3], + [getAsPaymentAddress('C'), 0.5] + ]), + 10n, + undefined, + () => 10n, + () => false + ) + ).rejects.toThrow(new InputSelectionError(InputSelectionFailure.UtxoBalanceInsufficient)); + }); +});