-
Notifications
You must be signed in to change notification settings - Fork 302
fix: tada withdrawal failure caused due to strict conditions #8186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ) { | ||
| find = true; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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', | ||
|
|
||
There was a problem hiding this comment.
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