Skip to content
Open
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
56 changes: 45 additions & 11 deletions modules/sdk-coin-dot/src/dot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,17 @@ import {
multisigTypes,
AuditDecryptedKeyParams,
verifyEddsaTssWalletAddress,
TxIntentMismatchRecipientError,
} from '@bitgo/sdk-core';
import { BaseCoin as StaticsBaseCoin, coins, PolkadotSpecNameType } from '@bitgo/statics';
import { Interface, KeyPair as DotKeyPair, Transaction, TransactionBuilderFactory, Utils } from './lib';
import {
Interface,
KeyPair as DotKeyPair,
NativeTransferBuilder,
Transaction,
TransactionBuilderFactory,
Utils,
} from './lib';
import '@polkadot/api-augment';
import { ApiPromise, WsProvider } from '@polkadot/api';
import { Material } from './lib/iface';
Expand Down Expand Up @@ -651,7 +659,7 @@ export class Dot extends BaseCoin {
}

async verifyTransaction(params: VerifyTransactionOptions): Promise<boolean> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dot verifyTransaction has been reverted here #7634

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#7652 revert of revert here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at the JSON for a DOT wallet and the base address for DOT is called rootAddress so I use that to retrieve the address here. I also typed the txBuilder, changed the generic errors to be TxIntentMismatchRecipientError where appropriate, and I rebased those changes on top of the revert of revert PR.

const { txPrebuild, txParams } = params;
const { txPrebuild, txParams, verification, wallet, reqId } = params;
if (!txParams) {
throw new Error('missing txParams');
}
Expand All @@ -665,7 +673,25 @@ export class Dot extends BaseCoin {
}

const factory = this.getBuilder();
const txBuilder = factory.from(txPrebuild.txHex) as any;
const txBuilder = factory.from(txPrebuild.txHex) as unknown as NativeTransferBuilder;

if (verification?.consolidationToBaseAddress) {
// Verify funds are sent to wallet's base address for consolidation
const baseAddress = wallet?.coinSpecific()?.rootAddress || wallet?.coinSpecific()?.baseAddress;
if (!baseAddress) {
throw new Error('Unable to determine base address for consolidation');
}
if (txBuilder['_to'] !== baseAddress) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you fetching it via ['_to'] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're not using the any type anymore for the txBuilder, and the _to property is protected in NativeTransferBuilder. This allows us to access this property.

throw new TxIntentMismatchRecipientError(
`Transaction destination address ${txBuilder['_to']} does not match wallet base address ${baseAddress}`,
reqId,
[txParams],
txPrebuild.txHex,
[{ address: txBuilder['_to'], amount: txBuilder['_amount'] }]
);
}
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's not return early.

}

if (txParams.recipients !== undefined) {
if (txParams.recipients.length === 0) {
Expand All @@ -678,17 +704,25 @@ export class Dot extends BaseCoin {
);
}

// validate recipient is same as txBuilder._to
if (txParams.recipients[0].address !== txBuilder._to) {
throw new Error(
`Recipient address ${txParams.recipients[0].address} does not match transaction destination address ${txBuilder._to}`
// validate recipient is same as txBuilder['_to']
if (txParams.recipients[0].address !== txBuilder['_to']) {
throw new TxIntentMismatchRecipientError(
`Recipient address ${txParams.recipients[0].address} does not match transaction destination address ${txBuilder['_to']}`,
reqId,
[txParams],
txPrebuild.txHex,
[{ address: txBuilder['_to'], amount: txBuilder['_amount'] }]
);
}

// and amount is same as txBuilder._amount
if (txParams.recipients[0].amount !== txBuilder._amount) {
throw new Error(
`Recipient amount ${txParams.recipients[0].amount} does not match transaction amount ${txBuilder._amount}`
// validate amount is same as txBuilder['_amount']
if (txParams.recipients[0].amount !== txBuilder['_amount']) {
throw new TxIntentMismatchRecipientError(
`Recipient amount ${txParams.recipients[0].amount} does not match transaction amount ${txBuilder['_amount']}`,
reqId,
[txParams],
txPrebuild.txHex,
[{ address: txBuilder['_to'], amount: txBuilder['_amount'] }]
);
}
}
Expand Down
47 changes: 47 additions & 0 deletions modules/sdk-coin-dot/test/unit/dot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,53 @@ describe('DOT:', function () {
const result = await basecoin.verifyTransaction({ txPrebuild, txParams });
assert.strictEqual(result, true);
});

it('should verify a valid consolidation transaction', async function () {
const mockedWallet = {
coinSpecific: () => ({
baseAddress: '5CZh773vKGwKFCYUjGc31AwXCbf7TPkavdeuk2XoujJMjbBD',
}),
};
const txPrebuild = {
txHex:
'0xa80a0300161b969b6b53ef81225feea3882284c778cd4a406d23215fcf492e83f75d42960b00204aa9d101eb600400000065900f001000000067f9723393ef76214df0118c34bbbd3dbebc8ed46a10973a8c969d48fe7598c9a7b7420ee3e4fe2b88da0fc42b30897e18d56d8b56a1934211d9de730cf96de300',
};

const result = await basecoin.verifyTransaction({
txPrebuild,
txParams: {},
wallet: mockedWallet as any,
verification: {
consolidationToBaseAddress: true,
},
});
assert.strictEqual(result, true);
});

it('should reject a consolidation transaction with invalid destination address', async function () {
const mockedWallet = {
coinSpecific: () => ({
baseAddress: '5DxD9nT16GQLrU6aB5pSS5VtxoZbVju3NHUCcawxZyZCTf74',
}),
};
const txPrebuild = {
txHex:
'0xa80a0300161b969b6b53ef81225feea3882284c778cd4a406d23215fcf492e83f75d42960b00204aa9d101eb600400000065900f001000000067f9723393ef76214df0118c34bbbd3dbebc8ed46a10973a8c969d48fe7598c9a7b7420ee3e4fe2b88da0fc42b30897e18d56d8b56a1934211d9de730cf96de300',
};

await basecoin
.verifyTransaction({
txPrebuild,
txParams: {},
wallet: mockedWallet as any,
verification: {
consolidationToBaseAddress: true,
},
})
.should.be.rejectedWith(
'Transaction destination address 5CZh773vKGwKFCYUjGc31AwXCbf7TPkavdeuk2XoujJMjbBD does not match wallet base address 5DxD9nT16GQLrU6aB5pSS5VtxoZbVju3NHUCcawxZyZCTf74'
);
});
});

describe('isWalletAddress', () => {
Expand Down