From b7c4b16a8ddacce3c19e9a2517841bd554a66f64 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 24 Sep 2021 15:05:38 -0400 Subject: [PATCH] feat: extra protection for AccountDelete transactions (#1626) * add deletion blockers check to autofill * add tests * add fail_hard: true * pass in account_objects response to error * only fail_hard for AccountDelete * reject promise instead of throwing error * fix rebase issue --- src/sugar/autofill.ts | 35 +++++++- src/sugar/submit.ts | 6 ++ test/client/autofill.ts | 84 +++++++++---------- .../fixtures/rippled/accountObjectsEmpty.json | 13 +++ test/fixtures/rippled/index.ts | 3 +- 5 files changed, 92 insertions(+), 49 deletions(-) create mode 100644 test/fixtures/rippled/accountObjectsEmpty.json diff --git a/src/sugar/autofill.ts b/src/sugar/autofill.ts index 58b270f7..db2730b1 100644 --- a/src/sugar/autofill.ts +++ b/src/sugar/autofill.ts @@ -2,8 +2,12 @@ import BigNumber from 'bignumber.js' import { xAddressToClassicAddress, isValidXAddress } from 'ripple-address-codec' import type { Client } from '..' -import { ValidationError } from '../errors' -import { AccountInfoRequest, LedgerRequest } from '../models/methods' +import { ValidationError, XrplError } from '../errors' +import { + AccountInfoRequest, + AccountObjectsRequest, + LedgerRequest, +} from '../models/methods' import { Transaction } from '../models/transactions' import setTransactionFlagsToNumber from '../models/utils/flags' import { xrpToDrops } from '../utils' @@ -46,6 +50,9 @@ async function autofill( if (tx.LastLedgerSequence == null) { promises.push(setLatestValidatedLedgerSequence(this, tx)) } + if (tx.TransactionType === 'AccountDelete') { + promises.push(checkAccountDeleteBlockers(this, tx)) + } return Promise.all(promises).then(() => tx) } @@ -193,4 +200,28 @@ async function setLatestValidatedLedgerSequence( tx.LastLedgerSequence = ledgerSequence + LEDGER_OFFSET } +async function checkAccountDeleteBlockers( + client: Client, + tx: Transaction, +): Promise { + const request: AccountObjectsRequest = { + command: 'account_objects', + account: tx.Account, + ledger_index: 'validated', + deletion_blockers_only: true, + } + const response = await client.request(request) + return new Promise((resolve, reject) => { + if (response.result.account_objects.length > 0) { + reject( + new XrplError( + `Account ${tx.Account} cannot be deleted; there are Escrows, PayChannels, RippleStates, or Checks associated with the account.`, + response.result.account_objects, + ), + ) + } + resolve() + }) +} + export default autofill diff --git a/src/sugar/submit.ts b/src/sugar/submit.ts index c46c8936..338580ca 100644 --- a/src/sugar/submit.ts +++ b/src/sugar/submit.ts @@ -51,6 +51,7 @@ async function submitSignedTransaction( const request: SubmitRequest = { command: 'submit', tx_blob: signedTxEncoded, + fail_hard: isAccountDelete(signedTransaction), } return this.request(request) } @@ -63,4 +64,9 @@ function isSigned(transaction: Transaction | string): boolean { ) } +function isAccountDelete(transaction: Transaction | string): boolean { + const tx = typeof transaction === 'string' ? decode(transaction) : transaction + return tx.TransactionType === 'AccountDelete' +} + export { submitTransaction, submitSignedTransaction } diff --git a/test/client/autofill.ts b/test/client/autofill.ts index ec887c65..7b021ccf 100644 --- a/test/client/autofill.ts +++ b/test/client/autofill.ts @@ -1,9 +1,16 @@ import { assert } from 'chai' -import { AccountDelete, EscrowFinish, Payment, Transaction } from 'xrpl-local' +import { + XrplError, + AccountDelete, + EscrowFinish, + Payment, + Transaction, +} from 'xrpl-local' import rippled from '../fixtures/rippled' import { setupClient, teardownClient } from '../setupClient' +import { assertRejects } from '../testUtils' const Fee = '10' const Sequence = 1432 @@ -71,6 +78,27 @@ describe('client.autofill', function () { assert.strictEqual(txResult.Sequence, 23) }) + it('should throw error if account deletion blockers exist', async function () { + this.mockRippled.addResponse('account_info', rippled.account_info.normal) + this.mockRippled.addResponse('ledger', rippled.ledger.normal) + this.mockRippled.addResponse('server_info', rippled.server_info.normal) + this.mockRippled.addResponse( + 'account_objects', + rippled.account_objects.normal, + ) + + const tx: AccountDelete = { + Account: 'rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn', + TransactionType: 'AccountDelete', + Destination: 'X7AcgcsBL6XDcUb289X4mJ8djcdyKaB5hJDWMArnXr61cqZ', + Fee, + Sequence, + LastLedgerSequence, + } + + await assertRejects(this.client.autofill(tx), XrplError) + }) + describe('when autofill Fee is missing', function () { it('should autofill Fee of a Transaction', async function () { const tx: Transaction = { @@ -80,17 +108,7 @@ describe('client.autofill', function () { Sequence, LastLedgerSequence, } - this.mockRippled.addResponse('server_info', { - status: 'success', - type: 'response', - result: { - info: { - validated_ledger: { - base_fee_xrp: 0.00001, - }, - }, - }, - }) + this.mockRippled.addResponse('server_info', rippled.server_info.normal) const txResult = await this.client.autofill(tx) assert.strictEqual(txResult.Fee, '12') @@ -108,19 +126,9 @@ describe('client.autofill', function () { } this.mockRippled.addResponse('account_info', rippled.account_info.normal) this.mockRippled.addResponse('ledger', rippled.ledger.normal) - this.mockRippled.addResponse('server_info', { - status: 'success', - type: 'response', - result: { - info: { - validated_ledger: { - base_fee_xrp: 0.00001, - }, - }, - }, - }) - const txResult = await this.client.autofill(tx) + this.mockRippled.addResponse('server_info', rippled.server_info.normal) + const txResult = await this.client.autofill(tx) assert.strictEqual(txResult.Fee, '399') }) @@ -132,17 +140,11 @@ describe('client.autofill', function () { } this.mockRippled.addResponse('account_info', rippled.account_info.normal) this.mockRippled.addResponse('ledger', rippled.ledger.normal) - this.mockRippled.addResponse('server_info', { - status: 'success', - type: 'response', - result: { - info: { - validated_ledger: { - base_fee_xrp: 0.00001, - }, - }, - }, - }) + this.mockRippled.addResponse('server_info', rippled.server_info.normal) + this.mockRippled.addResponse( + 'account_objects', + rippled.account_objects.empty, + ) const txResult = await this.client.autofill(tx) assert.strictEqual(txResult.Fee, '5000000') @@ -160,17 +162,7 @@ describe('client.autofill', function () { } this.mockRippled.addResponse('account_info', rippled.account_info.normal) this.mockRippled.addResponse('ledger', rippled.ledger.normal) - this.mockRippled.addResponse('server_info', { - status: 'success', - type: 'response', - result: { - info: { - validated_ledger: { - base_fee_xrp: 0.00001, - }, - }, - }, - }) + this.mockRippled.addResponse('server_info', rippled.server_info.normal) const txResult = await this.client.autofill(tx, 4) assert.strictEqual(txResult.Fee, '459') diff --git a/test/fixtures/rippled/accountObjectsEmpty.json b/test/fixtures/rippled/accountObjectsEmpty.json new file mode 100644 index 00000000..406c48d1 --- /dev/null +++ b/test/fixtures/rippled/accountObjectsEmpty.json @@ -0,0 +1,13 @@ +{ + "id": 1, + "status": "success", + "type": "response", + "result": { + "account": "r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59", + "account_objects": [], + "ledger_hash": + "053DF17D2289D1C4971C22F235BC1FCA7D4B3AE966F842E5819D0749E0B8ECD3", + "ledger_index": 14378733, + "validated": true + } +} diff --git a/test/fixtures/rippled/index.ts b/test/fixtures/rippled/index.ts index 3ae8b986..73cefcee 100644 --- a/test/fixtures/rippled/index.ts +++ b/test/fixtures/rippled/index.ts @@ -1,5 +1,6 @@ import normalAccountInfo from './accountInfo.json' import notfoundAccountInfo from './accountInfoNotFound.json' +import emptyAccountObjects from './accountObjectsEmpty.json' import normalAccountObjects from './accountObjectsNormal.json' import account_offers from './accountOffers' import normalAccountTx from './accountTx' @@ -133,7 +134,7 @@ const streams = { const account_objects = { normal: normalAccountObjects, - // notfound: notfoundAccountObjects + empty: emptyAccountObjects, } const account_info = {