Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion modules/sdk-coin-ada/src/ada.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ export class Ada extends BaseCoin {
for (const recipient of txParams.recipients) {
let find = false;
for (const output of explainedTx.outputs) {
if (recipient.address === output.address && recipient.amount === output.amount) {
if (
recipient.address === output.address &&
(recipient.amount === 'max' || BigInt(output.amount) >= BigInt(recipient.amount))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recipient.amount is the withdrawal amount. Max is a flag that we use internally to find out if it is a max withdrawal. amount can never be max. Please remove this condition

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BigInt(output.amount) >= BigInt(recipient.amount) this condition will allow withdrawals exceeding the expected amount and will not flag any buggy behaviour in build. As the issue is with the fee address deposits from the same enterprise as the fee address, let's check if the certs from the tx is 2 (one for sender and the other for gas tank), if so, BigInt(output.amount) >= BigInt(recipient.amount) can be validated

) {
find = true;
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/sdk-coin-ada/src/adaToken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class AdaToken extends Ada {
}
const multiAssets = output.multiAssets as CardanoWasm.MultiAsset;
const tokenQty = multiAssets.get_asset(policyScriptHash, assetName);
return tokenQty && tokenQty.to_str() === recipient.amount;
return tokenQty && (recipient.amount === 'max' || BigInt(tokenQty.to_str()) >= BigInt(recipient.amount));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the issue is with the fee address deposits which does not include tokens, this change is not required

});

if (!found) {
Expand Down
91 changes: 91 additions & 0 deletions modules/sdk-coin-ada/test/unit/ada.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,97 @@ describe('ADA', function () {
validTransaction.should.equal(true);
});

it('should succeed when output amount >= requested amount for any gas tank address (WIN-9022)', async () => {
// WIN-9022: gas tank transactions cause the server to absorb dust change into the
// output, so the actual output amount may be larger than requested. The >= check
// allows this for ANY address as long as the output sends at least what was requested.
const txPrebuild = newTxPrebuild();
const testCases = [
{ address: rawTx.outputAddress1.address, amount: '2000000' },
{ address: rawTx.outputAddress1.address, amount: '1' },
{ address: rawTx.outputAddress1.address, amount: '5000000' },
{ address: rawTx.outputAddress2.address, amount: '1' },
];
for (const { address, amount } of testCases) {
const txParams = {
recipients: [{ address, amount }],
};
const isTransactionVerified = await basecoin.verifyTransaction({
txParams,
txPrebuild,
verification: {},
});
isTransactionVerified.should.equal(true);
}
});

it('should fail when requested amount exceeds actual output amount', async () => {
const txPrebuild = newTxPrebuild();
const txParams = {
recipients: [
{
address: rawTx.outputAddress1.address,
amount: '9999999999',
},
],
};

await basecoin
.verifyTransaction({ txParams, txPrebuild, verification: {} })
.should.be.rejectedWith('cannot find recipient in expected output');
});

it('should succeed when recipient amount is max', async () => {
const txPrebuild = newTxPrebuild();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix/add test cases with new changes

const txParams = {
recipients: [
{
address: rawTx.outputAddress1.address,
amount: 'max',
},
],
};

const isTransactionVerified = await basecoin.verifyTransaction({
txParams,
txPrebuild,
verification: {},
});
isTransactionVerified.should.equal(true);
});

it('should fail when address does not match any output regardless of amount', async () => {
const txPrebuild = newTxPrebuild();
const txParams = {
recipients: [
{
address: 'addr_test1vrcx9yqtect97qlfq0hmttsd6ns38kvqw6lhcsyjwsh6zuqdpvg6u',
amount: '1',
},
],
};

await basecoin
.verifyTransaction({ txParams, txPrebuild, verification: {} })
.should.be.rejectedWith('cannot find recipient in expected output');
});

it('should fail when recipient amount is max but address does not match', async () => {
const txPrebuild = newTxPrebuild();
const txParams = {
recipients: [
{
address: '9f7b0675db59d19b4bd9c8c72eaabba75a9863d02b30115b8b3c3ca5c20f0254',
amount: 'max',
},
],
};

await basecoin
.verifyTransaction({ txParams, txPrebuild, verification: {} })
.should.be.rejectedWith('cannot find recipient in expected output');
});

it('should verify a valid consolidation transaction', async () => {
const consolidationTx = {
txRequestId: '1b5c79c5-ab7c-4f47-912b-de6a95fb0779',
Expand Down