Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix unfairness on proof size check estimation #2946

Merged
merged 18 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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({
noandrea marked this conversation as resolved.
Show resolved Hide resolved
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
Loading