From 2faa570673ea5215529cd80890955059410502c4 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Fri, 15 Nov 2024 13:16:42 +0200 Subject: [PATCH] fix: FeesTreasuryProportion is only applied to substrate based tx only (#3043) * fix: FeesTreasuryProportion is only applied to substrate based tx only * refactor: split test into 2 files * test: test 0% FeesTreasuryProportion * add substrate based tests * test: test for different cases * test: add extra check * fix: test calculation * style: fix formatting * fix: fix calculation for moonriver and moonbeam * fix: fix calculation * test: add with tip case --------- Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> --- runtime/moonbase/src/lib.rs | 12 +- runtime/moonbeam/src/lib.rs | 12 +- runtime/moonriver/src/lib.rs | 12 +- .../test-parameters-randomness.ts | 63 +++++ ...rameters-runtime-FeesTreasuryProportion.ts | 242 ++++++++++++++++++ .../test-parameters/test-parameters.ts | 71 +---- 6 files changed, 343 insertions(+), 69 deletions(-) create mode 100644 test/suites/dev/moonbase/test-parameters/test-parameters-randomness.ts create mode 100644 test/suites/dev/moonbase/test-parameters/test-parameters-runtime-FeesTreasuryProportion.ts diff --git a/runtime/moonbase/src/lib.rs b/runtime/moonbase/src/lib.rs index be2fd23073..7c699860f6 100644 --- a/runtime/moonbase/src/lib.rs +++ b/runtime/moonbase/src/lib.rs @@ -364,9 +364,9 @@ where mut fees_then_tips: impl Iterator>>, ) { if let Some(fees) = fees_then_tips.next() { - let treasury_perbill = + let treasury_proportion = runtime_params::dynamic_params::runtime_config::FeesTreasuryProportion::get(); - let treasury_part = treasury_perbill.deconstruct(); + let treasury_part = treasury_proportion.deconstruct(); let burn_part = Perbill::one().deconstruct() - treasury_part; let (_, to_treasury) = fees.ration(burn_part, treasury_part); // Balances pallet automatically burns dropped Credits by decreasing @@ -378,7 +378,7 @@ where // handle tip if there is one if let Some(tip) = fees_then_tips.next() { // for now we use the same burn/treasury strategy used for regular fees - let (_, to_treasury) = tip.ration(80, 20); + let (_, to_treasury) = tip.ration(burn_part, treasury_part); ResolveTo::, pallet_balances::Pallet>::on_unbalanced( to_treasury, ); @@ -391,7 +391,11 @@ where fn on_nonzero_unbalanced(amount: Credit>) { // Balances pallet automatically burns dropped Credits by decreasing // total_supply accordingly - let (_, to_treasury) = amount.ration(80, 20); + let treasury_proportion = + runtime_params::dynamic_params::runtime_config::FeesTreasuryProportion::get(); + let treasury_part = treasury_proportion.deconstruct(); + let burn_part = Perbill::one().deconstruct() - treasury_part; + let (_, to_treasury) = amount.ration(burn_part, treasury_part); ResolveTo::, pallet_balances::Pallet>::on_unbalanced(to_treasury); } } diff --git a/runtime/moonbeam/src/lib.rs b/runtime/moonbeam/src/lib.rs index a8ca7810a8..91635966c5 100644 --- a/runtime/moonbeam/src/lib.rs +++ b/runtime/moonbeam/src/lib.rs @@ -352,9 +352,9 @@ where mut fees_then_tips: impl Iterator>>, ) { if let Some(fees) = fees_then_tips.next() { - let treasury_perbill = + let treasury_proportion = runtime_params::dynamic_params::runtime_config::FeesTreasuryProportion::get(); - let treasury_part = treasury_perbill.deconstruct(); + let treasury_part = treasury_proportion.deconstruct(); let burn_part = Perbill::one().deconstruct() - treasury_part; let (_, to_treasury) = fees.ration(burn_part, treasury_part); // Balances pallet automatically burns dropped Credits by decreasing @@ -366,7 +366,7 @@ where // handle tip if there is one if let Some(tip) = fees_then_tips.next() { // for now we use the same burn/treasury strategy used for regular fees - let (_, to_treasury) = tip.ration(80, 20); + let (_, to_treasury) = tip.ration(burn_part, treasury_part); ResolveTo::, pallet_balances::Pallet>::on_unbalanced( to_treasury, ); @@ -379,7 +379,11 @@ where fn on_nonzero_unbalanced(amount: Credit>) { // Balances pallet automatically burns dropped Credits by decreasing // total_supply accordingly - let (_, to_treasury) = amount.ration(80, 20); + let treasury_proportion = + runtime_params::dynamic_params::runtime_config::FeesTreasuryProportion::get(); + let treasury_part = treasury_proportion.deconstruct(); + let burn_part = Perbill::one().deconstruct() - treasury_part; + let (_, to_treasury) = amount.ration(burn_part, treasury_part); ResolveTo::, pallet_balances::Pallet>::on_unbalanced(to_treasury); } } diff --git a/runtime/moonriver/src/lib.rs b/runtime/moonriver/src/lib.rs index ad68874966..bd9776892e 100644 --- a/runtime/moonriver/src/lib.rs +++ b/runtime/moonriver/src/lib.rs @@ -354,9 +354,9 @@ where mut fees_then_tips: impl Iterator>>, ) { if let Some(fees) = fees_then_tips.next() { - let treasury_perbill = + let treasury_proportion = runtime_params::dynamic_params::runtime_config::FeesTreasuryProportion::get(); - let treasury_part = treasury_perbill.deconstruct(); + let treasury_part = treasury_proportion.deconstruct(); let burn_part = Perbill::one().deconstruct() - treasury_part; let (_, to_treasury) = fees.ration(burn_part, treasury_part); // Balances pallet automatically burns dropped Credits by decreasing @@ -368,7 +368,7 @@ where // handle tip if there is one if let Some(tip) = fees_then_tips.next() { // for now we use the same burn/treasury strategy used for regular fees - let (_, to_treasury) = tip.ration(80, 20); + let (_, to_treasury) = tip.ration(burn_part, treasury_part); ResolveTo::, pallet_balances::Pallet>::on_unbalanced( to_treasury, ); @@ -381,7 +381,11 @@ where fn on_nonzero_unbalanced(amount: Credit>) { // Balances pallet automatically burns dropped Credits by decreasing // total_supply accordingly - let (_, to_treasury) = amount.ration(80, 20); + let treasury_proportion = + runtime_params::dynamic_params::runtime_config::FeesTreasuryProportion::get(); + let treasury_part = treasury_proportion.deconstruct(); + let burn_part = Perbill::one().deconstruct() - treasury_part; + let (_, to_treasury) = amount.ration(burn_part, treasury_part); ResolveTo::, pallet_balances::Pallet>::on_unbalanced(to_treasury); } } diff --git a/test/suites/dev/moonbase/test-parameters/test-parameters-randomness.ts b/test/suites/dev/moonbase/test-parameters/test-parameters-randomness.ts new file mode 100644 index 0000000000..f811d0df99 --- /dev/null +++ b/test/suites/dev/moonbase/test-parameters/test-parameters-randomness.ts @@ -0,0 +1,63 @@ +import { describeSuite, expect } from "@moonwall/cli"; +import { alith } from "@moonwall/util"; +import { parameterType, UNIT } from "./test-parameters"; + +describeSuite({ + id: "DTemp02", + title: "Parameters - Pallet Randomness", + foundationMethods: "dev", + testCases: ({ it, context, log }) => { + it({ + id: `T01 - PalletRandomness - Deposit - CustomTests`, + title: "Deposit parameter should only be accepted in bounds", + test: async () => { + const MIN = 1n * UNIT; + const MAX = 1000n * UNIT; + + // used as an acceptable value + const AVG = (MIN + MAX) / 2n; + + const param1 = parameterType(context, "PalletRandomness", "Deposit", MIN - 1n); + try { + await context.createBlock( + context + .polkadotJs() + .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param1.toU8a())) + .signAsync(alith), + { allowFailures: false } + ); + expect.fail("An extrinsic should not be created, since the parameter is invalid"); + } catch (error) { + expect(error.toString().toLowerCase()).to.contain("value out of bounds"); + } + + const param2 = parameterType(context, "PalletRandomness", "Deposit", MAX + 1n); + try { + await context.createBlock( + context + .polkadotJs() + .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param2.toU8a())) + .signAsync(alith), + { allowFailures: false } + ); + expect.fail("An extrinsic should not be created, since the parameter is invalid"); + } catch (error) { + expect(error.toString().toLowerCase()).to.contain("value out of bounds"); + } + + const param3 = parameterType(context, "PalletRandomness", "Deposit", AVG); + const res3 = await context.createBlock( + context + .polkadotJs() + .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param3.toU8a())) + .signAsync(alith), + { allowFailures: false } + ); + expect( + res3.result?.successful, + "An extrinsic should be created, since the parameter is valid" + ).to.be.true; + }, + }); + }, +}); diff --git a/test/suites/dev/moonbase/test-parameters/test-parameters-runtime-FeesTreasuryProportion.ts b/test/suites/dev/moonbase/test-parameters/test-parameters-runtime-FeesTreasuryProportion.ts new file mode 100644 index 0000000000..bd10311cc8 --- /dev/null +++ b/test/suites/dev/moonbase/test-parameters/test-parameters-runtime-FeesTreasuryProportion.ts @@ -0,0 +1,242 @@ +import { describeSuite, expect, TransactionTypes } from "@moonwall/cli"; +import { + alith, + baltathar, + BALTATHAR_ADDRESS, + createRawTransfer, + extractFee, + Perbill, + TREASURY_ACCOUNT, +} from "@moonwall/util"; +import { parameterType, UNIT } from "./test-parameters"; +import { BN } from "@polkadot/util"; + +interface TestCase { + proportion: Perbill; + + transfer_amount: bigint; + tipAmount: bigint; +} + +// Recreation on fees.ration(burn_part, treasury_part) +const split = (value: BN, part1: BN, part2: BN): [BN, BN] => { + const total = part1.add(part2); + if (total.eq(new BN(0)) || value.eq(new BN(0))) { + return [new BN(0), new BN(0)]; + } + const part1BN = value.mul(part1).div(total); + const part2BN = value.sub(part1BN); + return [part1BN, part2BN]; +}; + +describeSuite({ + id: "DTemp03", + title: "Parameters - RuntimeConfig", + foundationMethods: "dev", + testCases: ({ it, context, log }) => { + let testCounter = 0; + + const testCases: TestCase[] = [ + { + proportion: new Perbill(0), + transfer_amount: 10n * UNIT, + tipAmount: 1n * UNIT, + }, + { + proportion: new Perbill(1, 100), + transfer_amount: 1000n, + tipAmount: 100n, + }, + { + proportion: new Perbill(355, 1000), + transfer_amount: 5n * UNIT, + tipAmount: 111112n, + }, + { + proportion: new Perbill(400, 1000), + transfer_amount: 10n * UNIT, + tipAmount: 1n * UNIT, + }, + { + proportion: new Perbill(500, 1000), + transfer_amount: 10n * UNIT, + tipAmount: 1n * UNIT, + }, + { + proportion: new Perbill(963, 1000), + transfer_amount: 10n * UNIT, + tipAmount: 128n, + }, + { + proportion: new Perbill(99, 100), + transfer_amount: 10n * UNIT, + tipAmount: 3n, + }, + { + proportion: new Perbill(1, 1), + transfer_amount: 10n * UNIT, + tipAmount: 32n, + }, + ]; + + for (const t of testCases) { + const burnProportion = new Perbill(new BN(1e9).sub(t.proportion.value())); + + const treasuryPercentage = t.proportion.value().toNumber() / 1e7; + const burnPercentage = burnProportion.value().toNumber() / 1e7; + + const calcTreasuryIncrease = (feeWithTip: bigint, tip?: bigint): bigint => { + const issuanceDecrease = calcIssuanceDecrease(feeWithTip, tip); + const treasuryIncrease = feeWithTip - issuanceDecrease; + console.log("issuanceDecrease", issuanceDecrease.toString()); + console.log("treasuryIncrease", treasuryIncrease.toString()); + return treasuryIncrease; + }; + const calcIssuanceDecrease = (feeWithTip: bigint, tip?: bigint): bigint => { + const feeWithTipBN = new BN(feeWithTip.toString()); + const tipBN = new BN(tip?.toString() || "0"); + const feeWithoutTipBN = feeWithTipBN.sub(tipBN); + + const [burnFeePart, _treasuryFeePart] = split( + feeWithoutTipBN, + burnProportion.value(), + t.proportion.value() + ); + const [burnTipPart, _treasuryTipPart] = split( + tipBN, + burnProportion.value(), + t.proportion.value() + ); + + console.log("burn percentage", burnPercentage); + console.log("treasury percentage", treasuryPercentage); + console.log("feeWithTipBN", feeWithTipBN.toString()); + console.log("tipBN", tipBN.toString()); + console.log("feeWithoutTipBN", feeWithoutTipBN.toString()); + console.log("burnFeePart", burnFeePart.toString()); + console.log("treasuryFeePart", _treasuryFeePart.toString()); + console.log("burnTipPart", burnTipPart.toString()); + console.log("treasuryTipPart", _treasuryTipPart.toString()); + + return BigInt(burnFeePart.add(burnTipPart).toString()); + }; + + for (const txnType of TransactionTypes) { + it({ + id: `T${++testCounter}`, + title: + `Changing FeesTreasuryProportion to ${treasuryPercentage}% for Ethereum ` + + `${txnType} transfers`, + test: async () => { + const param = parameterType( + context, + "RuntimeConfig", + "FeesTreasuryProportion", + t.proportion.value() + ); + await context.createBlock( + context + .polkadotJs() + .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param.toU8a())) + .signAsync(alith), + { allowFailures: false } + ); + + const balBefore = await context.viem().getBalance({ address: TREASURY_ACCOUNT }); + const issuanceBefore = ( + await context.polkadotJs().query.balances.totalIssuance() + ).toBigInt(); + const { result } = await context.createBlock( + await createRawTransfer(context, BALTATHAR_ADDRESS, t.transfer_amount, { + type: txnType, + }) + ); + + const balAfter = await context.viem().getBalance({ address: TREASURY_ACCOUNT }); + const issuanceAfter = ( + await context.polkadotJs().query.balances.totalIssuance() + ).toBigInt(); + + const treasuryIncrease = balAfter - balBefore; + const issuanceDecrease = issuanceBefore - issuanceAfter; + const fee = extractFee(result?.events)!.amount.toBigInt(); + + expect( + treasuryIncrease + issuanceDecrease, + `Sum of TreasuryIncrease and IssuanceDecrease should be equal to the fees` + ).to.equal(fee); + + expect( + treasuryIncrease, + `${treasuryPercentage}% of the fees should go to treasury` + ).to.equal(calcTreasuryIncrease(fee)); + + expect(issuanceDecrease, `${burnPercentage}% of the fees should be burned`).to.equal( + calcIssuanceDecrease(fee) + ); + }, + }); + } + + for (const withTip of [false, true]) { + it({ + id: `T${++testCounter}`, + title: + `Changing FeesTreasuryProportion to ${treasuryPercentage}% for Substrate based` + + `transactions with ${withTip ? "" : "no "}tip`, + test: async () => { + const param = parameterType( + context, + "RuntimeConfig", + "FeesTreasuryProportion", + t.proportion.value() + ); + await context.createBlock( + context + .polkadotJs() + .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param.toU8a())) + .signAsync(alith), + { allowFailures: false } + ); + + const balanceBefore = await context.viem().getBalance({ address: TREASURY_ACCOUNT }); + const issuanceBefore = ( + await context.polkadotJs().query.balances.totalIssuance() + ).toBigInt(); + + const { result } = await context.createBlock( + context + .polkadotJs() + .tx.balances.transferKeepAlive(alith.address, t.transfer_amount) + .signAsync(baltathar, { tip: withTip ? t.tipAmount : 0n }), + { allowFailures: false } + ); + + const balanceAfter = await context.viem().getBalance({ address: TREASURY_ACCOUNT }); + const issuanceAfter = ( + await context.polkadotJs().query.balances.totalIssuance() + ).toBigInt(); + + const treasuryIncrease = balanceAfter - balanceBefore; + const issuanceDecrease = issuanceBefore - issuanceAfter; + const fee = extractFee(result?.events)!.amount.toBigInt(); + + expect( + treasuryIncrease + issuanceDecrease, + `Sum of TreasuryIncrease and IssuanceDecrease should be equal to the fees` + ).to.equal(fee); + + expect( + treasuryIncrease, + `${treasuryPercentage}% of the fees should go to treasury` + ).to.equal(calcTreasuryIncrease(fee, withTip ? t.tipAmount : undefined)); + + expect(issuanceDecrease, `${burnPercentage}% of the fees should be burned`).to.equal( + calcIssuanceDecrease(fee, withTip ? t.tipAmount : undefined) + ); + }, + }); + } + } + }, +}); diff --git a/test/suites/dev/moonbase/test-parameters/test-parameters.ts b/test/suites/dev/moonbase/test-parameters/test-parameters.ts index 46638a2be7..fb1fbef96a 100644 --- a/test/suites/dev/moonbase/test-parameters/test-parameters.ts +++ b/test/suites/dev/moonbase/test-parameters/test-parameters.ts @@ -1,14 +1,19 @@ import { describeSuite, DevModeContext, expect } from "@moonwall/cli"; import "@moonbeam-network/api-augment"; import { alith } from "@moonwall/util"; -import { fail } from "assert"; -const UNIT = 1_000_000_000_000_000_000n; + +export const UNIT = 1_000_000_000_000_000_000n; const RUNTIME = "MoonbaseRuntime"; const CRATE = "RuntimeParams"; const ALL_PARAMS = "DynamicParams"; -function parameterType(context: DevModeContext, module: string, name: string, value: unknown) { +export function parameterType( + context: DevModeContext, + module: string, + name: string, + value: unknown +) { const paramWrapper = context .polkadotJs() .createType(`${RUNTIME}${CRATE}${ALL_PARAMS}${module}Parameters`, { @@ -22,7 +27,7 @@ function parameterType(context: DevModeContext, module: string, name: string, va return runtimeParameter; } -function parameterKey(context: DevModeContext, module: string, name: string) { +export function parameterKey(context: DevModeContext, module: string, name: string) { const key = context .polkadotJs() .createType(`${RUNTIME}${CRATE}${ALL_PARAMS}${module}ParametersKey`, { @@ -42,6 +47,7 @@ describeSuite({ foundationMethods: "dev", testCases: ({ it, context, log }) => { let testCounter = 0; + function testParam(module: string, name: string, valueCreation: [string, unknown]) { it({ id: `T${testCounter++} - ${module} - ${name}`, @@ -82,60 +88,11 @@ describeSuite({ }); } + // Add all the parameters here to test against 2 test cases: + // 1. Parameters cannot by a normal user. + // 2. Parameters can be set by sudo. + testParam("RuntimeConfig", "FeesTreasuryProportion", ["Perbill", 200_000_000]); testParam("PalletRandomness", "Deposit", ["u128", UNIT * 100n]); - - it({ - id: `T${testCounter++} - PalletRandomness - Deposit - CustomTests`, - title: "Deposit parameter should only be accepted in bounds", - test: async () => { - const MIN = 1n * UNIT; - const MAX = 1000n * UNIT; - - // used as an acceptable value - const AVG = (MIN + MAX) / 2n; - - const param1 = parameterType(context, "PalletRandomness", "Deposit", MIN - 1n); - try { - await context.createBlock( - context - .polkadotJs() - .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param1.toU8a())) - .signAsync(alith), - { allowFailures: false } - ); - fail("An extrinsic should not be created, since the parameter is invalid"); - } catch (error) { - expect(error.toString().toLowerCase()).to.contain("value out of bounds"); - } - - const param2 = parameterType(context, "PalletRandomness", "Deposit", MAX + 1n); - try { - await context.createBlock( - context - .polkadotJs() - .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param2.toU8a())) - .signAsync(alith), - { allowFailures: false } - ); - fail("An extrinsic should not be created, since the parameter is invalid"); - } catch (error) { - expect(error.toString().toLowerCase()).to.contain("value out of bounds"); - } - - const param3 = parameterType(context, "PalletRandomness", "Deposit", AVG); - const res3 = await context.createBlock( - context - .polkadotJs() - .tx.sudo.sudo(context.polkadotJs().tx.parameters.setParameter(param3.toU8a())) - .signAsync(alith), - { allowFailures: false } - ); - expect( - res3.result?.successful, - "An extrinsic should be created, since the parameter is valid" - ).to.be.true; - }, - }); }, });