diff --git a/modules/sdk-coin-near/src/lib/fungibleTokenTransferBuilder.ts b/modules/sdk-coin-near/src/lib/fungibleTokenTransferBuilder.ts index c750c97ebc..79bbb41515 100644 --- a/modules/sdk-coin-near/src/lib/fungibleTokenTransferBuilder.ts +++ b/modules/sdk-coin-near/src/lib/fungibleTokenTransferBuilder.ts @@ -8,6 +8,7 @@ import { BaseCoin as CoinConfig } from '@bitgo/statics'; import { FT_TRANSFER, STORAGE_DEPOSIT } from './constants'; import { ContractCallWrapper } from './contractCallWrapper'; +import { AddressValidationError } from './errors'; import { StorageDepositInput } from './iface'; import { Transaction } from './transaction'; import { TransactionBuilder } from './transactionBuilder'; @@ -77,7 +78,9 @@ export class FungibleTokenTransferBuilder extends TransactionBuilder { * @param accountId the receiver account id */ public ftReceiverId(accountId: string): this { - utils.isValidAddress(accountId); + if (!accountId || !utils.isValidAddress(accountId)) { + throw new AddressValidationError(accountId); + } this.contractCallWrapper.args = { receiver_id: accountId }; return this; } diff --git a/modules/sdk-coin-near/src/lib/storageDepositTransferBuilder.ts b/modules/sdk-coin-near/src/lib/storageDepositTransferBuilder.ts index 37ec368b6c..107efbdacf 100644 --- a/modules/sdk-coin-near/src/lib/storageDepositTransferBuilder.ts +++ b/modules/sdk-coin-near/src/lib/storageDepositTransferBuilder.ts @@ -6,6 +6,7 @@ import { BuildTransactionError, TransactionType } from '@bitgo/sdk-core'; import { BaseCoin as CoinConfig } from '@bitgo/statics'; import { ContractCallWrapper } from './contractCallWrapper'; +import { AddressValidationError } from './errors'; import { Transaction } from './transaction'; import { TransactionBuilder } from './transactionBuilder'; import { STORAGE_DEPOSIT } from './constants'; @@ -66,7 +67,9 @@ export class StorageDepositTransferBuilder extends TransactionBuilder { * @param accountId the receiver account id */ public beneficiaryId(accountId: string): this { - utils.isValidAddress(accountId); + if (!accountId || !utils.isValidAddress(accountId)) { + throw new AddressValidationError(accountId); + } this.contractCallWrapper.args = { account_id: accountId }; return this; } diff --git a/modules/sdk-coin-near/src/lib/transactionBuilder.ts b/modules/sdk-coin-near/src/lib/transactionBuilder.ts index eb01fee92a..e2b175ba7d 100644 --- a/modules/sdk-coin-near/src/lib/transactionBuilder.ts +++ b/modules/sdk-coin-near/src/lib/transactionBuilder.ts @@ -163,7 +163,9 @@ export abstract class TransactionBuilder extends BaseTransactionBuilder { * @returns {TransactionBuilder} This transaction builder */ public receiverId(accountId: string): this { - utils.isValidAddress(accountId); + if (!accountId || !utils.isValidAddress(accountId)) { + throw new AddressValidationError(accountId); + } this._receiverId = accountId; return this; } diff --git a/modules/sdk-coin-near/src/near.ts b/modules/sdk-coin-near/src/near.ts index f4ca475aa0..b6f94895cc 100644 --- a/modules/sdk-coin-near/src/near.ts +++ b/modules/sdk-coin-near/src/near.ts @@ -830,6 +830,43 @@ export class Near extends BaseCoin { return availableBalance.toString(); } + /** + * Checks whether a NEAR account exists on-chain via the `view_account` RPC query. + * + * Implicit accounts (64-character lowercase hex, i.e. a raw ed25519 public key) are excluded + * from this check: an unfunded implicit account also reports UNKNOWN_ACCOUNT even though a + * transfer to it is a valid way to create it on-chain, so treating that as "does not exist" + * would incorrectly block legitimate transfers. + * + * @param {string} accountId the account id to check + * @returns {Promise} true if the account exists (or is an implicit account), false otherwise + */ + protected async doesAccountExist(accountId: string): Promise { + if (/^[0-9a-f]{64}$/.test(accountId)) { + return true; + } + const response = await this.getDataFromNode({ + payload: { + jsonrpc: '2.0', + id: 'dontcare', + method: 'query', + params: { + request_type: 'view_account', + finality: 'final', + account_id: accountId, + }, + }, + }); + const errorCause = response.body.error?.cause?.name; + if (errorCause === 'UNKNOWN_ACCOUNT') { + return false; + } + if (errorCause !== undefined) { + throw new Error(errorCause); + } + return true; + } + /** * Function to get the fungible token balance for an account * @param {String} accountId account for which the ft balance to be fetched @@ -1086,6 +1123,17 @@ export class Near extends BaseCoin { if (!totalAmount.isEqualTo(explainedTx.outputAmount) && txParams.type !== 'enabletoken') { throw new Error('Tx total amount does not match with expected total amount field'); } + + // enabletoken recipients are the wallet's own storage/token contract accounts, not + // user-supplied destinations, so existence checking doesn't apply to them. + if (txParams.type !== 'enabletoken') { + for (const recipient of txParams.recipients) { + const exists = await this.doesAccountExist(recipient.address); + if (!exists) { + throw new Error(`Recipient account '${recipient.address}' does not exist on NEAR`); + } + } + } } if (params.verification?.consolidationToBaseAddress) { diff --git a/modules/sdk-coin-near/test/unit/near.ts b/modules/sdk-coin-near/test/unit/near.ts index a4e603cf31..9f763d78fd 100644 --- a/modules/sdk-coin-near/test/unit/near.ts +++ b/modules/sdk-coin-near/test/unit/near.ts @@ -443,6 +443,11 @@ describe('NEAR:', function () { }); it('should verify activate staking transaction', async function () { + const sandBox = sinon.createSandbox(); + sandBox.stub(Near.prototype, 'getDataFromNode' as keyof Near).resolves({ + status: 200, + body: { result: { amount: '0', locked: '0', storage_usage: 0 } }, + }); const txBuilder = factory.getStakingActivateBuilder(); txBuilder .amount(amount) @@ -467,6 +472,7 @@ describe('NEAR:', function () { }; const validTransaction = await basecoin.verifyTransaction({ txParams, txPrebuild }); validTransaction.should.equal(true); + sandBox.restore(); }); it('should verify deactivate staking transaction', async function () { @@ -518,6 +524,119 @@ describe('NEAR:', function () { validTransaction.should.equal(true); }); + describe('Recipient account existence check', () => { + const namedReceiverId = 'somereceiver.testnet'; + const sandBox = sinon.createSandbox(); + + afterEach(() => { + sandBox.restore(); + }); + + const buildNamedTransferTxPrebuild = async () => { + const txBuilder = factory.getTransferBuilder(); + txBuilder + .sender(accounts.account1.address, accounts.account1.publicKey) + .nonce(BigInt(1)) + .receiverId(namedReceiverId) + .recentBlockHash(blockHash.block1) + .amount(amount); + txBuilder.sign({ key: accounts.account1.secretKey }); + const tx = await txBuilder.build(); + return { txHex: tx.toBroadcastFormat() }; + }; + + it('should succeed when the named recipient account exists on-chain', async function () { + const stub = sandBox.stub(Near.prototype, 'getDataFromNode' as keyof Near).resolves({ + status: 200, + body: { result: { amount: '0', locked: '0', storage_usage: 0 } }, + }); + const txPrebuild = await buildNamedTransferTxPrebuild(); + const txParams = { + recipients: [{ address: namedReceiverId, amount }], + }; + const validTransaction = await basecoin.verifyTransaction({ txParams, txPrebuild }); + validTransaction.should.equal(true); + stub.calledOnce.should.equal(true); + }); + + it('should fail when the named recipient account does not exist on-chain', async function () { + sandBox.stub(Near.prototype, 'getDataFromNode' as keyof Near).resolves({ + status: 200, + body: { error: { cause: { name: 'UNKNOWN_ACCOUNT' } } }, + }); + const txPrebuild = await buildNamedTransferTxPrebuild(); + const txParams = { + recipients: [{ address: namedReceiverId, amount }], + }; + await basecoin + .verifyTransaction({ txParams, txPrebuild }) + .should.be.rejectedWith(`Recipient account '${namedReceiverId}' does not exist on NEAR`); + }); + + it('should propagate other RPC errors from the existence check', async function () { + sandBox.stub(Near.prototype, 'getDataFromNode' as keyof Near).resolves({ + status: 200, + body: { error: { cause: { name: 'SOME_OTHER_ERROR' } } }, + }); + const txPrebuild = await buildNamedTransferTxPrebuild(); + const txParams = { + recipients: [{ address: namedReceiverId, amount }], + }; + await basecoin.verifyTransaction({ txParams, txPrebuild }).should.be.rejectedWith('SOME_OTHER_ERROR'); + }); + + it('should not call the RPC for implicit (hex) recipient accounts', async function () { + const stub = sandBox.stub(Near.prototype, 'getDataFromNode' as keyof Near); + const txPrebuild = newTxPrebuild(); + const txParams = newTxParams(); + const validTransaction = await basecoin.verifyTransaction({ txParams, txPrebuild }); + validTransaction.should.equal(true); + stub.called.should.equal(false); + }); + + it('should also check existence for staking transaction recipients', async function () { + const stub = sandBox.stub(Near.prototype, 'getDataFromNode' as keyof Near).resolves({ + status: 200, + body: { result: { amount: '0', locked: '0', storage_usage: 0 } }, + }); + const txBuilder = factory.getStakingActivateBuilder(); + txBuilder + .amount(amount) + .gas(gas) + .sender(accounts.account1.address, accounts.account1.publicKey) + .receiverId(validatorContractAddress) + .recentBlockHash(blockHash.block1) + .nonce(BigInt(1)); + txBuilder.sign({ key: accounts.account1.secretKey }); + const tx = await txBuilder.build(); + const txPrebuild = { txHex: tx.toBroadcastFormat() }; + const txParams = { + recipients: [{ address: validatorContractAddress, amount: '1000000' }], + }; + const validTransaction = await basecoin.verifyTransaction({ txParams, txPrebuild }); + validTransaction.should.equal(true); + stub.calledOnce.should.equal(true); + }); + + it('should not call the RPC for enabletoken recipients', async function () { + const stub = sandBox.stub(Near.prototype, 'getDataFromNode' as keyof Near); + const txPrebuild = { txHex: testData.rawTx.selfStorageDeposit.unsigned }; + const txParams = { + type: 'enabletoken', + recipients: [ + { + address: testData.accounts.account1.address, + amount: '0', + tokenName: 'tnear:tnep24dp', + }, + ], + }; + const validTransaction = await basecoin.verifyTransaction({ txParams, txPrebuild }); + validTransaction.should.equal(true); + stub.called.should.equal(false); + }); + }); + it('should verify a spoofed consolidation transaction', async function () { // Set up wallet data const walletData = { diff --git a/modules/sdk-coin-near/test/unit/transactionBuilder/fungibleTokenTransferBuilder.ts b/modules/sdk-coin-near/test/unit/transactionBuilder/fungibleTokenTransferBuilder.ts index 32fb40b02a..eacc4d7f71 100644 --- a/modules/sdk-coin-near/test/unit/transactionBuilder/fungibleTokenTransferBuilder.ts +++ b/modules/sdk-coin-near/test/unit/transactionBuilder/fungibleTokenTransferBuilder.ts @@ -1,3 +1,4 @@ +import assert from 'assert'; import should from 'should'; import * as base58 from 'bs58'; @@ -47,6 +48,37 @@ describe('Near: Fungible Token Transfer Builder', () => { return txBuilder; }; + describe('should fail', function () { + it('should throw when ftReceiverId is empty', () => { + const txBuilder = factory.getFungibleTokenTransferBuilder(); + assert.throws(() => txBuilder.ftReceiverId(''), /The address '' is not a well-formed near address/); + }); + + it('should throw when ftReceiverId has spaces', () => { + const txBuilder = factory.getFungibleTokenTransferBuilder(); + assert.throws( + () => txBuilder.ftReceiverId(testData.accounts.errorsAccounts.address1), + /The address 'not ok' is not a well-formed near address/ + ); + }); + + it('should throw when ftReceiverId has invalid characters', () => { + const txBuilder = factory.getFungibleTokenTransferBuilder(); + assert.throws( + () => txBuilder.ftReceiverId(testData.accounts.errorsAccounts.address4), + /The address '\$\$\$' is not a well-formed near address/ + ); + }); + + it('should throw when ftReceiverId is too long', () => { + const txBuilder = factory.getFungibleTokenTransferBuilder(); + assert.throws( + () => txBuilder.ftReceiverId(testData.accounts.errorsAccounts.address5), + /is not a well-formed near address/ + ); + }); + }); + describe('fungible token builder environment', function () { it('should select the right network', function () { should.equal(factory.getFungibleTokenTransferBuilder().coinName(), coinNameTest); diff --git a/modules/sdk-coin-near/test/unit/transactionBuilder/storageDepositTransferBuilder.ts b/modules/sdk-coin-near/test/unit/transactionBuilder/storageDepositTransferBuilder.ts index 94a4c6a334..20167bb826 100644 --- a/modules/sdk-coin-near/test/unit/transactionBuilder/storageDepositTransferBuilder.ts +++ b/modules/sdk-coin-near/test/unit/transactionBuilder/storageDepositTransferBuilder.ts @@ -1,3 +1,4 @@ +import assert from 'assert'; import should from 'should'; import * as base58 from 'bs58'; @@ -44,6 +45,37 @@ describe('Near: Storage Deposit Transfer Builder', () => { return txBuilder; }; + describe('should fail', function () { + it('should throw when beneficiaryId is empty', () => { + const txBuilder = factory.getStorageDepositTransferBuilder(); + assert.throws(() => txBuilder.beneficiaryId(''), /The address '' is not a well-formed near address/); + }); + + it('should throw when beneficiaryId has spaces', () => { + const txBuilder = factory.getStorageDepositTransferBuilder(); + assert.throws( + () => txBuilder.beneficiaryId(testData.accounts.errorsAccounts.address1), + /The address 'not ok' is not a well-formed near address/ + ); + }); + + it('should throw when beneficiaryId has invalid characters', () => { + const txBuilder = factory.getStorageDepositTransferBuilder(); + assert.throws( + () => txBuilder.beneficiaryId(testData.accounts.errorsAccounts.address4), + /The address '\$\$\$' is not a well-formed near address/ + ); + }); + + it('should throw when beneficiaryId is too long', () => { + const txBuilder = factory.getStorageDepositTransferBuilder(); + assert.throws( + () => txBuilder.beneficiaryId(testData.accounts.errorsAccounts.address5), + /is not a well-formed near address/ + ); + }); + }); + describe('fungible token builder environment', function () { it('should select the right network', function () { should.equal(factory.getStorageDepositTransferBuilder().coinName(), coinNameTest); diff --git a/modules/sdk-coin-near/test/unit/transactionBuilder/transactionBuilder.ts b/modules/sdk-coin-near/test/unit/transactionBuilder/transactionBuilder.ts index 8a68345d3c..cbc0e0a3ba 100644 --- a/modules/sdk-coin-near/test/unit/transactionBuilder/transactionBuilder.ts +++ b/modules/sdk-coin-near/test/unit/transactionBuilder/transactionBuilder.ts @@ -123,4 +123,35 @@ describe('NEAR Transaction Builder', async () => { ); } }); + + describe('should fail when receiverId is invalid', async () => { + it('should throw on empty receiverId', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws(() => txBuilder.receiverId(''), /The address '' is not a well-formed near address/); + }); + + it('should throw on receiverId with spaces', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws( + () => txBuilder.receiverId(testData.accounts.errorsAccounts.address1), + /The address 'not ok' is not a well-formed near address/ + ); + }); + + it('should throw on receiverId with invalid characters', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws( + () => txBuilder.receiverId(testData.accounts.errorsAccounts.address4), + /The address '\$\$\$' is not a well-formed near address/ + ); + }); + + it('should throw on receiverId that is too long', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws( + () => txBuilder.receiverId(testData.accounts.errorsAccounts.address5), + /is not a well-formed near address/ + ); + }); + }); }); diff --git a/modules/sdk-coin-near/test/unit/transactionBuilder/transferBuilder.ts b/modules/sdk-coin-near/test/unit/transactionBuilder/transferBuilder.ts index 93b87b8fed..8cca06a976 100644 --- a/modules/sdk-coin-near/test/unit/transactionBuilder/transferBuilder.ts +++ b/modules/sdk-coin-near/test/unit/transactionBuilder/transferBuilder.ts @@ -1,3 +1,4 @@ +import assert from 'assert'; import { getBuilderFactory } from '../getBuilderFactory'; import { KeyPair } from '../../../src'; import should from 'should'; @@ -8,6 +9,37 @@ import * as base58 from 'bs58'; describe('Near Transfer Builder', () => { const factory = getBuilderFactory('tnear'); + describe('Fail', () => { + it('should throw on empty receiverId', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws(() => txBuilder.receiverId(''), /The address '' is not a well-formed near address/); + }); + + it('should throw on receiverId with spaces', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws( + () => txBuilder.receiverId(testData.accounts.errorsAccounts.address1), + /The address 'not ok' is not a well-formed near address/ + ); + }); + + it('should throw on receiverId with invalid characters', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws( + () => txBuilder.receiverId(testData.accounts.errorsAccounts.address4), + /The address '\$\$\$' is not a well-formed near address/ + ); + }); + + it('should throw on receiverId that is too long', () => { + const txBuilder = factory.getTransferBuilder(); + assert.throws( + () => txBuilder.receiverId(testData.accounts.errorsAccounts.address5), + /is not a well-formed near address/ + ); + }); + }); + describe('Succeed', () => { it('build a transfer tx unsigned', async () => { const txBuilder = factory.getTransferBuilder();