Skip to content

Commit

Permalink
Handled UTXO sums over u64 (#1946)
Browse files Browse the repository at this point in the history
  • Loading branch information
@SebastienGllmt
SebastienGllmt committed Mar 2, 2021
1 parent 8546381 commit 437bcf2
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 13 deletions.
68 changes: 57 additions & 11 deletions app/api/ada/transactions/shelley/transactions.js
View file
Expand Up @@ -11,6 +11,7 @@ import type {
import type { RemoteUnspentOutput, } from '../../lib/state-fetch/types';
import {
NotEnoughMoneyToSendError,
AssetOverflowError,
NoOutputsError,
} from '../../../common/errors';

Expand Down Expand Up @@ -97,6 +98,15 @@ export function sendAllUnsignedTx(
};
}

const AddInputResult = Object.freeze({
VALID: 0,
// not worth the fee of adding it to input
TOO_SMALL: 1,
// token would overflow if added
OVERFLOW: 2,
// doesn't contribute to target
NO_NEED: 3,
});
function addUtxoInput(
txBuilder: RustModule.WalletV4.TransactionBuilder,
remaining: void | {|
Expand All @@ -109,16 +119,37 @@ function addUtxoInput(
protocolParams: {|
networkId: number,
|},
): boolean {
): $Values<typeof AddInputResult> {
const wasmAddr = normalizeToAddress(input.receiver);
if (wasmAddr == null) {
throw new Error(`${nameof(addUtxoInput)} input not a valid Shelley address`);
}
const txInput = utxoToTxInput(input);
const wasmAmount = cardanoValueFromRemoteFormat(input);

const skipInput: void => boolean = () => {
if (remaining == null) return false;
const skipOverflow: void => $Values<typeof AddInputResult> = () => {
/**
* UTXOs can only contain at most u64 of a value
* so if the sum of UTXO inputs for a tx > u64
* it can cause the tx to fail (due to overflow) in the output / change
*
* This can be addressed by splitting up a tx to use multiple outputs / multiple change
* and this just requires more ADA to cover the min UTXO of these added inputs
* but as a simple solution for now, we just block > u64 inputs of any token
* This isn't a great workaround since it means features like sendAll may end up not sending all
*/
const currentInputSum = txBuilder
.get_explicit_input()
.checked_add(txBuilder.get_implicit_input());
try {
currentInputSum.checked_add(wasmAmount);
} catch (e) {
return AddInputResult.OVERFLOW;
}
return AddInputResult.VALID;
}
const skipInput: void => $Values<typeof AddInputResult> = () => {
if (remaining == null) return skipOverflow();

const defaultEntry = {
defaultNetworkId: protocolParams.networkId,
Expand All @@ -132,6 +163,7 @@ function addUtxoInput(
const includedTargets = remainingTokens.nonDefaultEntries().filter(
entry => tokenSetInInput.has(entry.identifier)
);

if (remainingTokens.getDefaultEntry().amount.gt(0) && new BigNumber(input.amount).gt(0)) {
includedTargets.push(remainingTokens.getDefaultEntry());
}
Expand All @@ -140,7 +172,7 @@ function addUtxoInput(
// due to refunds in Cardano
// so we still want to add the input in this case even if we don't care about the coins in it
if (includedTargets.length === 0 && remaining.hasInput) {
return true;
return AddInputResult.NO_NEED;
}

const onlyDefaultEntry = (
Expand All @@ -157,19 +189,24 @@ function addUtxoInput(
).to_str()
);
if (feeForInput.gt(input.amount)) {
return true;
return AddInputResult.TOO_SMALL;
}
}
return false;

return skipOverflow();
}

const skipResult = skipInput();
if (skipResult !== AddInputResult.VALID) {
return skipResult;
}
if (skipInput()) { return false; }

txBuilder.add_input(
wasmAddr,
txInput,
wasmAmount
);
return true;
return AddInputResult.VALID;
}

export function sendAllUnsignedTxFromUtxo(
Expand Down Expand Up @@ -202,14 +239,20 @@ export function sendAllUnsignedTxFromUtxo(
protocolParams.keyDeposit,
);
txBuilder.set_ttl(absSlotNumber.plus(defaultTtlOffset).toNumber());

for (const input of allUtxos) {
addUtxoInput(
if (addUtxoInput(
txBuilder,
undefined,
input,
false,
{ networkId: protocolParams.networkId }
);
) === AddInputResult.OVERFLOW) {
// for the send all case, prefer to throw an error
// instead of skipping inputs that would cause an error
// otherwise leads to unexpected cases like wallet migration leaving some UTXO behind
throw new AssetOverflowError();
}
}

if(metadata !== undefined){
Expand Down Expand Up @@ -521,7 +564,7 @@ export function newAdaUnsignedTxFromUtxo(
true,
{ networkId: protocolParams.networkId },
);
if (!added) continue;
if (added !== AddInputResult.VALID) continue;

usedUtxos.push(utxo);
}
Expand All @@ -544,6 +587,9 @@ export function newAdaUnsignedTxFromUtxo(
);
if (forceChange) {
if (changeAdaAddr == null) throw new NoOutputsError();
if (!enoughInput) {
throw new NotEnoughMoneyToSendError();
}
const difference = currentInputSum.checked_sub(output);
const minimumNeededForChange = minRequiredForChange(
txBuilder,
Expand Down
94 changes: 94 additions & 0 deletions app/api/ada/transactions/shelley/transactions.test.js
View file
Expand Up @@ -16,6 +16,7 @@ import {
import {
NotEnoughMoneyToSendError,
NoOutputsError,
AssetOverflowError,
} from '../../../common/errors';

import {
Expand Down Expand Up @@ -106,6 +107,22 @@ const genSampleUtxos: void => Array<RemoteUnspentOutput> = () => [
name: testAssetId.split('.')[1],
}],
},
{
amount: '10000001',
receiver: Buffer.from(RustModule.WalletV4.Address.from_bech32(
// external addr 0, staking key 0
'addr1q8gpjmyy8zk9nuza24a0f4e7mgp9gd6h3uayp0rqnjnkl54v4dlyj0kwfs0x4e38a7047lymzp37tx0y42glslcdtzhqphf76y'
).to_bytes()).toString('hex'),
tx_hash: '86e36b6a65d82c9dcc0370b0ee3953aee579db0b837753306405c28a74de5550',
tx_index: 0,
utxo_id: '86e36b6a65d82c9dcc0370b0ee3953aee579db0b837753306405c28a74de55500',
assets: [{
amount: '18446744073709551615', // max u64
assetId: testAssetId,
policyId: testAssetId.split('.')[0],
name: testAssetId.split('.')[1],
}],
},
];

const genSampleAdaAddresses: void => Array<{| ...Address, ...Addressing |}> = () => [
Expand Down Expand Up @@ -540,6 +557,83 @@ describe('Create unsigned TX from UTXO', () => {
undefined,
)).not.toThrow(NotEnoughMoneyToSendError);
});

it('Should fail when insufficient ADA when forcing change', () => {
const sampleUtxos = genSampleUtxos();
const sampleAdaAddresses = genSampleAdaAddresses();
const output = new MultiToken(
[{
// bigger than input including fees
amount: new BigNumber(1900001),
identifier: defaultIdentifier,
networkId: network.NetworkId,
}],
{
defaultIdentifier,
defaultNetworkId: network.NetworkId,
}
);

expect(() => newAdaUnsignedTxFromUtxo(
[{
address: byronAddrToHex('Ae2tdPwUPEZKX8N2TjzBXLy5qrecnQUniTd2yxE8mWyrh2djNpUkbAtXtP4'),
amount: output,
}],
sampleAdaAddresses[0],
[sampleUtxos[4]],
new BigNumber(0),
getProtocolParams(),
[],
[],
true,
)).toThrow(NotEnoughMoneyToSendError);
});

it('Should fail when sending all where sum of tokens > 2^64', () => {
const sampleUtxos = genSampleUtxos();
expect(() => sendAllUnsignedTxFromUtxo(
{
address: byronAddrToHex('Ae2tdPwUPEZKX8N2TjzBXLy5qrecnQUniTd2yxE8mWyrh2djNpUkbAtXtP4'),
},
[sampleUtxos[4], sampleUtxos[5]],
new BigNumber(0),
getProtocolParams(),
undefined,
)).toThrow(AssetOverflowError);
});
it('Should skip inputs when sending where sum of tokens > 2^64', () => {
const sampleUtxos = genSampleUtxos();
const sampleAdaAddresses = genSampleAdaAddresses();
const output = new MultiToken(
[{
amount: new BigNumber(19001),
identifier: defaultIdentifier,
networkId: network.NetworkId,
}],
{
defaultIdentifier,
defaultNetworkId: network.NetworkId,
}
);

const result = newAdaUnsignedTxFromUtxo(
[{
address: byronAddrToHex('Ae2tdPwUPEZKX8N2TjzBXLy5qrecnQUniTd2yxE8mWyrh2djNpUkbAtXtP4'),
amount: output,
}],
sampleAdaAddresses[0],
[sampleUtxos[4], sampleUtxos[5]],
new BigNumber(0),
getProtocolParams(),
[],
[],
true,
);
// one of the inputs skipped to keep <= u64
expect(result.senderUtxos.length).toEqual(1);
});
});

describe('Create unsigned TX from addresses', () => {
Expand Down
12 changes: 12 additions & 0 deletions app/api/common/errors.js
View file
Expand Up @@ -48,6 +48,10 @@ const messages = defineMessages({
id: 'api.errors.NotEnoughMoneyToSendError',
defaultMessage: '!!!Not enough funds to make this transaction.',
},
assetOverflowError: {
id: 'api.errors.assetOverflowError',
defaultMessage: '!!!Maximum value of a token inside a UTXO exceeded (overflow).',
},
updateAdaWalletError: {
id: 'api.errors.updateAdaWalletError',
defaultMessage: '!!!Error while updating ada wallet.',
Expand Down Expand Up @@ -272,6 +276,14 @@ export class NotEnoughMoneyToSendError extends LocalizableError {
});
}
}
export class AssetOverflowError extends LocalizableError {
constructor() {
super({
id: messages.assetOverflowError.id,
defaultMessage: messages.assetOverflowError.defaultMessage || '',
});
}
}

export class GetBalanceError extends LocalizableError {
constructor() {
Expand Down
4 changes: 2 additions & 2 deletions features/transactions.feature
View file
Expand Up @@ -428,7 +428,7 @@ Feature: Send transaction
When I go to the send transaction screen

And I open the token selection dropdown
And I select token "d2719...f696e"
And I select token "nicoin"

And I fill the form:
| address | amount |
Expand Down Expand Up @@ -457,7 +457,7 @@ Feature: Send transaction
When I go to the send transaction screen

And I open the token selection dropdown
And I select token "d2719...f696e"
And I select token "nicoin"

And I click on "Send all" checkbox
And I fill the address of the form:
Expand Down

0 comments on commit 437bcf2

Please sign in to comment.