From 56723e153fee93ab41adb4be271e7e7e0aa30607 Mon Sep 17 00:00:00 2001 From: Phu Pham Date: Thu, 24 Oct 2024 18:23:24 -0400 Subject: [PATCH 1/7] FeeSettings updates --- .ci-config/rippled.cfg | 6 ++++++ packages/xrpl/HISTORY.md | 3 +++ packages/xrpl/src/sugar/autofill.ts | 18 ++++++++---------- .../xrpl/test/integration/requests/fee.test.ts | 16 +++++++++++----- .../transactions/xchainClaim.test.ts | 7 ++++++- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/.ci-config/rippled.cfg b/.ci-config/rippled.cfg index 291f87b730..54d36021d8 100644 --- a/.ci-config/rippled.cfg +++ b/.ci-config/rippled.cfg @@ -178,3 +178,9 @@ PriceOracle fixEmptyDID fixXChainRewardRounding fixPreviousTxnID + +# # This section can be used to simulate various FeeSettings scenarios for rippled node in standalone mode +# [voting] +reference_fee = 200 # 200 drops +account_reserve = 20000000 # 20 XRP +owner_reserve = 5000000 # 5 XRP diff --git a/packages/xrpl/HISTORY.md b/packages/xrpl/HISTORY.md index e6deb8457b..b635a16801 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -7,6 +7,9 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr ### Added * parseTransactionFlags as a utility function in the xrpl package to streamline transactions flags-to-map conversion +### Fixed +* Remove hard-coded reference to 10 drops as the reference transaction cost. Ensure tests passed for all transaction fee scenarios and `AMMCreate` transaction fee calculation is correct in case `owner_reserve` increases. + ## 4.0.0 (2024-07-15) ### BREAKING CHANGES diff --git a/packages/xrpl/src/sugar/autofill.ts b/packages/xrpl/src/sugar/autofill.ts index ca0757611d..05178933b2 100644 --- a/packages/xrpl/src/sugar/autofill.ts +++ b/packages/xrpl/src/sugar/autofill.ts @@ -230,13 +230,13 @@ export async function setNextValidSequenceNumber( } /** - * Fetches the account deletion fee from the server state using the provided client. + * Fetches the owner reserve fee from the server state using the provided client. * * @param client - The client object used to make the request. * @returns A Promise that resolves to the account deletion fee as a BigNumber. * @throws {Error} Throws an error if the account deletion fee cannot be fetched. */ -async function fetchAccountDeleteFee(client: Client): Promise { +async function fetchOwnerReserveFee(client: Client): Promise { const response = await client.request({ command: 'server_state' }) const fee = response.result.state.validated_ledger?.reserve_inc @@ -260,7 +260,6 @@ export async function calculateFeePerTransactionType( tx: Transaction, signersCount = 0, ): Promise { - // netFee is usually 0.00001 XRP (10 drops) const netFeeXRP = await getFeeXrp(client) const netFeeDrops = xrpToDrops(netFeeXRP) let baseFee = new BigNumber(netFeeDrops) @@ -268,7 +267,7 @@ export async function calculateFeePerTransactionType( // EscrowFinish Transaction with Fulfillment if (tx.TransactionType === 'EscrowFinish' && tx.Fulfillment != null) { const fulfillmentBytesSize: number = Math.ceil(tx.Fulfillment.length / 2) - // 10 drops × (33 + (Fulfillment size in bytes / 16)) + // BaseFee × (33 + (Fulfillment size in bytes / 16)) const product = new BigNumber( // eslint-disable-next-line @typescript-eslint/no-magic-numbers -- expected use of magic numbers scaleValue(netFeeDrops, 33 + fulfillmentBytesSize / 16), @@ -280,22 +279,21 @@ export async function calculateFeePerTransactionType( tx.TransactionType === 'AccountDelete' || tx.TransactionType === 'AMMCreate' ) { - baseFee = await fetchAccountDeleteFee(client) + baseFee = await fetchOwnerReserveFee(client) } /* * Multi-signed Transaction - * 10 drops × (1 + Number of Signatures Provided) + * BaseFee × (1 + Number of Signatures Provided) */ if (signersCount > 0) { baseFee = BigNumber.sum(baseFee, scaleValue(netFeeDrops, 1 + signersCount)) } const maxFeeDrops = xrpToDrops(client.maxFeeXRP) - const totalFee = - tx.TransactionType === 'AccountDelete' - ? baseFee - : BigNumber.min(baseFee, maxFeeDrops) + const totalFee = ['AccountDelete', 'AMMCreate'].includes(tx.TransactionType) + ? baseFee + : BigNumber.min(baseFee, maxFeeDrops) // Round up baseFee and return it as a string // eslint-disable-next-line no-param-reassign, @typescript-eslint/no-magic-numbers -- param reassign is safe, base 10 magic num diff --git a/packages/xrpl/test/integration/requests/fee.test.ts b/packages/xrpl/test/integration/requests/fee.test.ts index 3ba557e8bf..40936f514e 100644 --- a/packages/xrpl/test/integration/requests/fee.test.ts +++ b/packages/xrpl/test/integration/requests/fee.test.ts @@ -1,7 +1,9 @@ +import BigNumber from 'bignumber.js' import { assert } from 'chai' import omit from 'lodash/omit' -import { FeeRequest } from '../../../src' +import { FeeRequest, xrpToDrops } from '../../../src' +import getFeeXrp from '../../../src/sugar/getFeeXrp' import serverUrl from '../serverUrl' import { setupClient, @@ -26,6 +28,8 @@ describe('fee', function () { const request: FeeRequest = { command: 'fee', } + const referenceFee = xrpToDrops(await getFeeXrp(testContext.client, 1)) + const MEDIAN_FEE_MULTIPLIER = 500 const response = await testContext.client.request(request) const expected = { id: 0, @@ -33,10 +37,12 @@ describe('fee', function () { current_ledger_size: '0', current_queue_size: '0', drops: { - base_fee: '10', - median_fee: '5000', - minimum_fee: '10', - open_ledger_fee: '10', + base_fee: referenceFee, + median_fee: new BigNumber(referenceFee) + .times(MEDIAN_FEE_MULTIPLIER) + .toString(), + minimum_fee: referenceFee, + open_ledger_fee: referenceFee, }, expected_ledger_size: '1000', ledger_current_index: 2925, diff --git a/packages/xrpl/test/integration/transactions/xchainClaim.test.ts b/packages/xrpl/test/integration/transactions/xchainClaim.test.ts index 1987ceec4a..3465ed4724 100644 --- a/packages/xrpl/test/integration/transactions/xchainClaim.test.ts +++ b/packages/xrpl/test/integration/transactions/xchainClaim.test.ts @@ -9,6 +9,7 @@ import { XChainCreateClaimID, xrpToDrops, } from '../../../src' +import getFeeXrp from '../../../src/sugar/getFeeXrp' import serverUrl from '../serverUrl' import { setupBridge, @@ -39,6 +40,7 @@ describe('XChainCreateBridge', function () { const destination = await generateFundedWallet(testContext.client) const otherChainSource = Wallet.generate() const amount = xrpToDrops(10) + const netFee = xrpToDrops(await getFeeXrp(testContext.client)) const claimIdTx: XChainCreateClaimID = { TransactionType: 'XChainCreateClaimID', @@ -103,7 +105,10 @@ describe('XChainCreateBridge', function () { ) assert.equal( finalBalance, - initialBalance + Number(amount) - Number(signatureReward) - 12, + initialBalance + + Number(amount) - + Number(signatureReward) - + Number(netFee), "The destination's balance should not change yet", ) }, From d96931b90af3fefd6464f8e9fe05cf71ff389ccd Mon Sep 17 00:00:00 2001 From: Phu Pham Date: Fri, 25 Oct 2024 11:13:06 -0400 Subject: [PATCH 2/7] small fix --- .ci-config/rippled.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci-config/rippled.cfg b/.ci-config/rippled.cfg index 54d36021d8..6976c24637 100644 --- a/.ci-config/rippled.cfg +++ b/.ci-config/rippled.cfg @@ -180,7 +180,7 @@ fixXChainRewardRounding fixPreviousTxnID # # This section can be used to simulate various FeeSettings scenarios for rippled node in standalone mode -# [voting] +[voting] reference_fee = 200 # 200 drops account_reserve = 20000000 # 20 XRP owner_reserve = 5000000 # 5 XRP From bb3156d3cbd82ffa6cc7d7c5cb9061939e7a9fbf Mon Sep 17 00:00:00 2001 From: Phu Pham Date: Fri, 25 Oct 2024 12:26:50 -0400 Subject: [PATCH 3/7] fix payment test --- .../integration/transactions/payment.test.ts | 20 +++++++++++++------ packages/xrpl/test/integration/utils.ts | 14 +++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/xrpl/test/integration/transactions/payment.test.ts b/packages/xrpl/test/integration/transactions/payment.test.ts index 391ab317b2..76b72a7a0a 100644 --- a/packages/xrpl/test/integration/transactions/payment.test.ts +++ b/packages/xrpl/test/integration/transactions/payment.test.ts @@ -7,7 +7,11 @@ import { teardownClient, type XrplIntegrationTestContext, } from '../setup' -import { generateFundedWallet, testTransaction } from '../utils' +import { + fetchAccountReserveFee, + generateFundedWallet, + testTransaction, +} from '../utils' // how long before each test case times out const TIMEOUT = 20000 @@ -15,7 +19,8 @@ const TIMEOUT = 20000 describe('Payment', function () { let testContext: XrplIntegrationTestContext let paymentTx: Payment - const AMOUNT = '10000000' + let amount: string + const DEFAULT_AMOUNT = '10000000' // This wallet is used for DeliverMax related tests let senderWallet: Wallet @@ -25,7 +30,7 @@ describe('Payment', function () { paymentTx = { TransactionType: 'Payment', Account: senderWallet.classicAddress, - Amount: AMOUNT, + Amount: amount, Destination: 'rfkE1aSy9G8Upk4JssnwBxhEv5p4mn2KTy', } }) @@ -33,6 +38,9 @@ describe('Payment', function () { beforeAll(async () => { testContext = await setupClient(serverUrl) senderWallet = await generateFundedWallet(testContext.client) + // Make sure the amount sent satisfies minimum reserve requirement to fund an account. + amount = + (await fetchAccountReserveFee(testContext.client)) ?? DEFAULT_AMOUNT }) afterAll(async () => teardownClient(testContext)) @@ -60,7 +68,7 @@ describe('Payment', function () { ) assert.equal(result.result.engine_result_code, 0) - assert.equal((result.result.tx_json as Payment).Amount, AMOUNT) + assert.equal((result.result.tx_json as Payment).Amount, amount) }, TIMEOUT, ) @@ -80,7 +88,7 @@ describe('Payment', function () { ) assert.equal(result.result.engine_result_code, 0) - assert.equal((result.result.tx_json as Payment).Amount, AMOUNT) + assert.equal((result.result.tx_json as Payment).Amount, amount) }, TIMEOUT, ) @@ -98,7 +106,7 @@ describe('Payment', function () { ) assert.equal(result.result.engine_result_code, 0) - assert.equal((result.result.tx_json as Payment).Amount, AMOUNT) + assert.equal((result.result.tx_json as Payment).Amount, amount) }, TIMEOUT, ) diff --git a/packages/xrpl/test/integration/utils.ts b/packages/xrpl/test/integration/utils.ts index 87f4529acb..4bc42f3906 100644 --- a/packages/xrpl/test/integration/utils.ts +++ b/packages/xrpl/test/integration/utils.ts @@ -1,3 +1,4 @@ +import BigNumber from 'bignumber.js' import { assert } from 'chai' import omit from 'lodash/omit' import throttle from 'lodash/throttle' @@ -444,3 +445,16 @@ export async function createAMMPool(client: Client): Promise<{ asset2, } } + +export async function fetchAccountReserveFee( + client: Client, +): Promise { + const response = await client.request({ command: 'server_state' }) + const fee = response.result.state.validated_ledger?.reserve_inc + + if (fee == null) { + return null + } + + return new BigNumber(fee).dp(0, BigNumber.ROUND_CEIL).toString(10) +} From 80a6d1f9eae2c82675b5fef090922a3f1e85bd2c Mon Sep 17 00:00:00 2001 From: Phu Pham Date: Fri, 25 Oct 2024 12:33:45 -0400 Subject: [PATCH 4/7] typo fix --- packages/xrpl/test/integration/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/xrpl/test/integration/utils.ts b/packages/xrpl/test/integration/utils.ts index 4bc42f3906..d8618e0291 100644 --- a/packages/xrpl/test/integration/utils.ts +++ b/packages/xrpl/test/integration/utils.ts @@ -450,7 +450,7 @@ export async function fetchAccountReserveFee( client: Client, ): Promise { const response = await client.request({ command: 'server_state' }) - const fee = response.result.state.validated_ledger?.reserve_inc + const fee = response.result.state.validated_ledger?.reserve_base if (fee == null) { return null From d907ce53c7d3b327eb6c894ff5cbb256c7a8cfe6 Mon Sep 17 00:00:00 2001 From: Phu Pham Date: Tue, 12 Nov 2024 15:03:15 -0500 Subject: [PATCH 5/7] nit changes --- .ci-config/rippled.cfg | 2 +- packages/xrpl/src/sugar/autofill.ts | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/.ci-config/rippled.cfg b/.ci-config/rippled.cfg index 6976c24637..ee584f4daa 100644 --- a/.ci-config/rippled.cfg +++ b/.ci-config/rippled.cfg @@ -179,7 +179,7 @@ fixEmptyDID fixXChainRewardRounding fixPreviousTxnID -# # This section can be used to simulate various FeeSettings scenarios for rippled node in standalone mode +# This section can be used to simulate various FeeSettings scenarios for rippled node in standalone mode [voting] reference_fee = 200 # 200 drops account_reserve = 20000000 # 20 XRP diff --git a/packages/xrpl/src/sugar/autofill.ts b/packages/xrpl/src/sugar/autofill.ts index 05178933b2..9df09576e2 100644 --- a/packages/xrpl/src/sugar/autofill.ts +++ b/packages/xrpl/src/sugar/autofill.ts @@ -233,8 +233,8 @@ export async function setNextValidSequenceNumber( * Fetches the owner reserve fee from the server state using the provided client. * * @param client - The client object used to make the request. - * @returns A Promise that resolves to the account deletion fee as a BigNumber. - * @throws {Error} Throws an error if the account deletion fee cannot be fetched. + * @returns A Promise that resolves to the owner reserve fee as a BigNumber. + * @throws {Error} Throws an error if the owner reserve fee cannot be fetched. */ async function fetchOwnerReserveFee(client: Client): Promise { const response = await client.request({ command: 'server_state' }) @@ -275,10 +275,11 @@ export async function calculateFeePerTransactionType( baseFee = product.dp(0, BigNumber.ROUND_CEIL) } - if ( - tx.TransactionType === 'AccountDelete' || - tx.TransactionType === 'AMMCreate' - ) { + const isSpecialTxCost = ['AccountDelete', 'AMMCreate'].includes( + tx.TransactionType, + ) + + if (isSpecialTxCost) { baseFee = await fetchOwnerReserveFee(client) } @@ -291,12 +292,12 @@ export async function calculateFeePerTransactionType( } const maxFeeDrops = xrpToDrops(client.maxFeeXRP) - const totalFee = ['AccountDelete', 'AMMCreate'].includes(tx.TransactionType) + const totalFee = isSpecialTxCost ? baseFee : BigNumber.min(baseFee, maxFeeDrops) // Round up baseFee and return it as a string - // eslint-disable-next-line no-param-reassign, @typescript-eslint/no-magic-numbers -- param reassign is safe, base 10 magic num + // eslint-disable-next-line no-param-reassign, @typescript-eslint/no-magic-numbers, require-atomic-updates -- safe reassignment. tx.Fee = totalFee.dp(0, BigNumber.ROUND_CEIL).toString(10) } From a36a8f0ea5384ce2baf97cdd5da78260102e39ea Mon Sep 17 00:00:00 2001 From: Phu Pham Date: Tue, 12 Nov 2024 15:40:53 -0500 Subject: [PATCH 6/7] remove unnecessary baseFee roundup --- packages/xrpl/src/sugar/autofill.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/xrpl/src/sugar/autofill.ts b/packages/xrpl/src/sugar/autofill.ts index 9df09576e2..31c5087a2c 100644 --- a/packages/xrpl/src/sugar/autofill.ts +++ b/packages/xrpl/src/sugar/autofill.ts @@ -268,11 +268,10 @@ export async function calculateFeePerTransactionType( if (tx.TransactionType === 'EscrowFinish' && tx.Fulfillment != null) { const fulfillmentBytesSize: number = Math.ceil(tx.Fulfillment.length / 2) // BaseFee × (33 + (Fulfillment size in bytes / 16)) - const product = new BigNumber( + baseFee = new BigNumber( // eslint-disable-next-line @typescript-eslint/no-magic-numbers -- expected use of magic numbers scaleValue(netFeeDrops, 33 + fulfillmentBytesSize / 16), ) - baseFee = product.dp(0, BigNumber.ROUND_CEIL) } const isSpecialTxCost = ['AccountDelete', 'AMMCreate'].includes( From 309e3c2e8f4c118f81f4b3240d18cc265e2167c2 Mon Sep 17 00:00:00 2001 From: Phu Pham Date: Tue, 12 Nov 2024 17:17:26 -0500 Subject: [PATCH 7/7] update contributing instruction --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 38b08ab0fc..9f2d1b54ec 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,7 +64,7 @@ From the top-level xrpl.js folder (one level above `packages`), run the followin ```bash npm install # sets up the rippled standalone Docker container - you can skip this step if you already have it set up -docker run -p 6006:6006 --interactive -t --volume $PWD/.ci-config:/opt/ripple/etc/ --platform linux/amd64 rippleci/rippled:2.0.0-b4 /opt/ripple/bin/rippled -a --conf /opt/ripple/etc/rippled.cfg +docker run -p 6006:6006 --interactive -t --volume $PWD/.ci-config:/opt/ripple/etc/ --platform linux/amd64 rippleci/rippled:2.2.0-b3 /opt/ripple/bin/rippled -a --conf /opt/ripple/etc/rippled.cfg npm run build npm run test:integration ```