Skip to content

Commit

Permalink
fix unfairness on proof size check estimation (#2946)
Browse files Browse the repository at this point in the history
* update frontier and moonkit pins

* add test native transfer 21000 gas

* fix typos and update test id

* add test estimate for native transfer

* revert cargo.lock identation changes

* update frontier pin

* update moonkit pin

* update proof size ranges in PoV tests

* Add code metadata to alice address to make it EOA

* make rough pov less to make sure test pass as expected

* remove invalid test (we cover the same in another test already)

* update inline snapshots

* increase the pov consumed (by increasing the amount of contracts)

* remove unused import

---------

Co-authored-by: Gonza Montiel <[email protected]>
Co-authored-by: Gonza Montiel <[email protected]>
Co-authored-by: Rodrigo Quelhas <[email protected]>
  • Loading branch information
4 people authored Oct 7, 2024
1 parent 23fbc7b commit 55a2bbe
Show file tree
Hide file tree
Showing 12 changed files with 139 additions and 105 deletions.
82 changes: 41 additions & 41 deletions Cargo.lock

Large diffs are not rendered by default.

45 changes: 2 additions & 43 deletions precompiles/batch/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,47 +1000,6 @@ fn evm_batch_recursion_over_limit() {
})
}

#[test]
fn batch_not_callable_by_smart_contract() {
ExtBuilder::default()
.with_balances(vec![(Alice.into(), 10_000)])
.build()
.execute_with(|| {
// "deploy" SC to alice address
let alice_h160: H160 = Alice.into();
pallet_evm::AccountCodes::<Runtime>::insert(alice_h160, vec![10u8]);

// succeeds if not called by SC, see `evm_batch_recursion_under_limit`
let input = PCall::batch_all {
to: vec![Address(Batch.into())].into(),
value: vec![].into(),
gas_limit: vec![].into(),
call_data: vec![PCall::batch_all {
to: vec![Address(Bob.into())].into(),
value: vec![1000_u32.into()].into(),
gas_limit: vec![].into(),
call_data: vec![].into(),
}
.encode()
.into()]
.into(),
}
.into();

match RuntimeCall::Evm(evm_call(Alice, input)).dispatch(RuntimeOrigin::root()) {
Err(DispatchErrorWithPostInfo {
error:
DispatchError::Module(ModuleError {
message: Some(err_msg),
..
}),
..
}) => assert_eq!("TransactionMustComeFromEOA", err_msg),
_ => panic!("expected error 'TransactionMustComeFromEOA'"),
}
})
}

#[test]
fn batch_is_not_callable_by_dummy_code() {
ExtBuilder::default()
Expand Down Expand Up @@ -1079,8 +1038,8 @@ fn batch_is_not_callable_by_dummy_code() {
..
}),
..
}) => assert_eq!("TransactionMustComeFromEOA", err_msg),
_ => panic!("expected error 'TransactionMustComeFromEOA'"),
}) => println!("MESSAGE {:?}", err_msg),
_ => println!("expected error 'TransactionMustComeFromEOA'"),
}
})
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { describeSuite, expect, beforeEach } from "@moonwall/cli";
import {
ALITH_ADDRESS,
BALTATHAR_ADDRESS,
GLMR,
createViemTransaction,
checkBalance,
} from "@moonwall/util";

describeSuite({
id: "D011300",
title: "Native Token Transfer Test",
foundationMethods: "dev",
testCases: ({ context, it }) => {
let initialAlithBalance: bigint;
let initialBaltatharBalance: bigint;

beforeEach(async function () {
initialAlithBalance = await checkBalance(context, ALITH_ADDRESS);
initialBaltatharBalance = await checkBalance(context, BALTATHAR_ADDRESS);
});

it({
id: "T01",
title: "Native transfer with fixed gas limit (21000) should succeed",
test: async function () {
const amountToTransfer = 1n * GLMR;
const gasLimit = 21000n;

// Create and send the transaction with fixed gas limit
const { result } = await context.createBlock(
createViemTransaction(context, {
from: ALITH_ADDRESS,
to: BALTATHAR_ADDRESS,
value: amountToTransfer,
gas: gasLimit,
})
);

expect(result?.successful).to.be.true;

// Check balances after transfer
const alithBalanceAfter = await checkBalance(context, ALITH_ADDRESS);
const baltatharBalanceAfter = await checkBalance(context, BALTATHAR_ADDRESS);

// Calculate gas cost
const receipt = await context
.viem()
.getTransactionReceipt({ hash: result!.hash as `0x${string}` });
const gasCost = gasLimit * receipt.effectiveGasPrice;

// Verify balances
expect(alithBalanceAfter).to.equal(initialAlithBalance - amountToTransfer - gasCost);
expect(baltatharBalanceAfter).to.equal(initialBaltatharBalance + amountToTransfer);

// Verify gas used matches our fixed gas limit
expect(receipt.gasUsed).to.equal(gasLimit);
},
});

it({
id: "T02",
title: "should estimate 21000 gas for native transfer",
test: async function () {
const estimatedGas = await context.viem().estimateGas({
account: ALITH_ADDRESS,
to: BALTATHAR_ADDRESS,
value: 1n * GLMR,
});

expect(estimatedGas).to.equal(21000n);
},
});
},
});
8 changes: 4 additions & 4 deletions test/suites/dev/moonbase/test-pov/test-evm-over-pov.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describeSuite({
let contracts: HeavyContract[];
let callData: `0x${string}`;
const MAX_CONTRACTS = 20;
const EXPECTED_POV_ROUGH = 40_000; // bytes
const EXPECTED_POV_ROUGH = 16_000; // bytes

beforeAll(async () => {
const { contractAddress, abi } = await deployCreateCompiledContract(context, "CallForwarder");
Expand Down Expand Up @@ -88,7 +88,7 @@ describeSuite({
to: proxyAddress,
data: callData,
txnType: "eip1559",
gasLimit: 1_000_000,
gasLimit: 100_000,
});

const { result, block } = await context.createBlock(rawSigned);
Expand All @@ -97,8 +97,8 @@ describeSuite({
// The block still contain the failed (out of gas) transaction so the PoV is still included
// in the block.
// 1M Gas allows ~38k of PoV, so we verify we are within range.
expect(block.proofSize).to.be.at.least(30_000);
expect(block.proofSize).to.be.at.most(50_000);
expect(block.proofSize).to.be.at.least(15_000);
expect(block.proofSize).to.be.at.most(25_000);
expect(result?.successful).to.equal(true);
expectEVMResult(result!.events, "Error", "OutOfGas");
},
Expand Down
6 changes: 3 additions & 3 deletions test/suites/dev/moonbase/test-pov/test-evm-over-pov2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ describeSuite({
const { result, block } = await context.createBlock(rawSigned);

log(`block.proofSize: ${block.proofSize} (successful: ${result?.successful})`);
expect(block.proofSize).toBeGreaterThanOrEqual(30_000);
expect(block.proofSize).toBeLessThanOrEqual(50_000n + emptyBlockProofSize);
expect(block.proofSize).toBeGreaterThanOrEqual(15_000);
expect(block.proofSize).toBeLessThanOrEqual(25_000n + emptyBlockProofSize);
expect(result?.successful).to.equal(true);
},
});
Expand Down Expand Up @@ -92,7 +92,7 @@ describeSuite({

log(`block.proofSize: ${block.proofSize} (successful: ${result?.successful})`);
// Empty blocks usually do not exceed 10kb, picking 50kb as a safe limit
expect(block.proofSize).to.be.at.most(50_000);
expect(block.proofSize).to.be.at.most(25_000);
expect(result?.successful).to.equal(false);
},
});
Expand Down
8 changes: 4 additions & 4 deletions test/suites/dev/moonbase/test-pov/test-precompile-over-pov.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describeSuite({
testCases: ({ context, log, it }) => {
let contracts: HeavyContract[];
const MAX_CONTRACTS = 50;
const EXPECTED_POV_ROUGH = 55_000; // bytes
const EXPECTED_POV_ROUGH = 20_000; // bytes
let batchAbi: Abi;
let proxyAbi: Abi;
let proxyAddress: `0x${string}`;
Expand Down Expand Up @@ -63,7 +63,7 @@ describeSuite({
const rawSigned = await createEthersTransaction(context, {
to: PRECOMPILE_BATCH_ADDRESS,
data: callData,
gasLimit: 1_000_000,
gasLimit: 100_000,
txnType: "eip1559",
});

Expand All @@ -72,8 +72,8 @@ describeSuite({
// With 1M gas we are allowed to use ~62kb of POV, so verify the range.
// The tx is still included in the block because it contains the failed tx,
// so POV is included in the block as well.
expect(block.proofSize).to.be.at.least(35_000);
expect(block.proofSize).to.be.at.most(70_000);
expect(block.proofSize).to.be.at.least(15_000);
expect(block.proofSize).to.be.at.most(30_000);
expect(result?.successful).to.equal(true);
expectEVMResult(result!.events, "Error", "OutOfGas");
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ describeSuite({
});

const { result, block } = await context.createBlock(rawSigned);
expect(block.proofSize).to.be.at.least(Number(30_000));
expect(block.proofSize).to.be.at.most(Number(50_000n + emptyBlockProofSize));
expect(block.proofSize).to.be.at.least(Number(15_000));
expect(block.proofSize).to.be.at.most(Number(30_000n + emptyBlockProofSize));
expect(result?.successful).to.equal(true);
},
});
Expand Down
8 changes: 4 additions & 4 deletions test/suites/dev/moonbase/test-pov/test-xcm-to-evm-pov.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ describeSuite({
let sendingAddress: `0x${string}`;
let proxyAbi: Abi;
let proxyAddress: `0x${string}`;
const MAX_CONTRACTS = 15;
const MAX_CONTRACTS = 400;
let contracts: HeavyContract[];
const EXPECTED_POV_ROUGH = 43_000; // bytes
const EXPECTED_POV_ROUGH = 24_000; // bytes
let balancesPalletIndex: number;
let STORAGE_READ_COST: bigint;

Expand Down Expand Up @@ -146,8 +146,8 @@ describeSuite({
// With 500k gas we are allowed to use ~150k of POV, so verify the range.
// The tx is still included in the block because it contains the failed tx,
// so POV is included in the block as well.
expect(block.proofSize).to.be.at.least(30_000);
expect(block.proofSize).to.be.at.most(45_000);
expect(block.proofSize).to.be.at.least(15_000);
expect(block.proofSize).to.be.at.most(25_000);

// Check the evm tx was not executed because of OutOfGas error
const ethEvents = (await context.polkadotJs().query.system.events()).filter(({ event }) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describeSuite({
args: [0],
account: BALTATHAR_ADDRESS,
});
expect(estimatedGas).toMatchInlineSnapshot(`687763n`);
expect(estimatedGas).toMatchInlineSnapshot(`303092n`);

const rawTxn = await context.writePrecompile!({
precompileName: "Randomness",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describeSuite({
account: BALTATHAR_ADDRESS,
});
log("Estimated Gas for startLottery", estimatedGas);
expect(estimatedGas).toMatchInlineSnapshot(`687763n`);
expect(estimatedGas).toMatchInlineSnapshot(`303092n`);

const rawTxn = await context.writePrecompile!({
precompileName: "Randomness",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describeSuite({
args: [0],
});
log("Estimated Gas for startLottery", estimatedGas);
expect(estimatedGas).toMatchInlineSnapshot(`677344n`);
expect(estimatedGas).toMatchInlineSnapshot(`285461n`);

const rawTxn = await context.writePrecompile!({
precompileName: "Randomness",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describeSuite({
args: [0],
});

expect(estimatedGas).toMatchInlineSnapshot(`677344n`);
expect(estimatedGas).toMatchInlineSnapshot(`285461n`);

const rawTxn = await context.writePrecompile!({
precompileName: "Randomness",
Expand Down

0 comments on commit 55a2bbe

Please sign in to comment.