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

EVM + Weight v2 support #1039

Merged
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
4bab80e
wip `record_extarnal_cost`
tgmichel Apr 11, 2023
416d618
Add support for external cost recording from the EVM
tgmichel Apr 13, 2023
24c91b5
Add refund external cost support
tgmichel Apr 14, 2023
3dff83e
Separate dispatchables with weight limit
tgmichel Apr 17, 2023
3ace0af
introduce tbd EthereumWeigher
tgmichel Apr 17, 2023
6deaacb
Validate gas to weight conversion
tgmichel Apr 19, 2023
74708f5
Add static opcode recording with temp costs
tgmichel Apr 20, 2023
8ea977b
Add optional `transaction_len` parameter to `GasWeightMapping`
tgmichel Apr 20, 2023
dc95f98
Account for pallet and call indexes
tgmichel Apr 20, 2023
ac5ed2d
Rollback `transaction_len` param, use MAX_POV_SIZE ratio instead
tgmichel Apr 21, 2023
45c9e25
wip tests
tgmichel Apr 25, 2023
dc33686
Add `uncached_account_code_proof_size_accounting_works` test
tgmichel Apr 26, 2023
312146f
wip tests
tgmichel Apr 26, 2023
88c7c7c
Temp remove static external cost accounting
tgmichel Apr 27, 2023
163b0c5
fix build
tgmichel Apr 27, 2023
a17e331
Temp remove static external cost accounting 2
tgmichel Apr 27, 2023
8cc4568
warning cleanup
tgmichel Apr 27, 2023
3646639
fmt
tgmichel Apr 27, 2023
87e5f6f
clippy
tgmichel Apr 27, 2023
cc4e173
taplo
tgmichel Apr 27, 2023
eb2afed
Add `evm-with-weight-limit` to ci
tgmichel Apr 27, 2023
7814419
Temp set evm fork + update Cargo.lock
tgmichel Apr 27, 2023
255f663
Merge branch 'master' into tgm-record-external-costs
tgmichel Apr 27, 2023
00ffebc
handle `code_hash`
tgmichel Apr 27, 2023
5ab6432
taplo
tgmichel Apr 27, 2023
4bb5e70
fmt
tgmichel Apr 27, 2023
538c072
Handle `transact_with_weight_limit` as self contained
tgmichel Apr 28, 2023
7686543
fix ts tests
tgmichel Apr 28, 2023
e12afd7
suggestion
tgmichel Apr 28, 2023
cd77d2d
remove precompile test
tgmichel Apr 28, 2023
91ce2ed
some suggestions
tgmichel Apr 28, 2023
d6d7a00
remove `transact_with_weight_limit`
tgmichel Apr 28, 2023
83e228a
configurable `MaxPovSize`
tgmichel May 3, 2023
c369476
test pov size constants
tgmichel May 3, 2023
296cb2a
accessed storage overlayed cache
tgmichel May 4, 2023
c8138af
`new_from_weight_limit` suggestion
tgmichel May 4, 2023
34be73b
remove unnecessary check
tgmichel May 4, 2023
44c93c0
warnings cleanup
tgmichel May 5, 2023
17abbd3
set constant gas limit max pov size ratio
tgmichel May 5, 2023
3572aad
check state version for suicide
tgmichel May 5, 2023
4bbbe3d
just completely remove suicide accounting
tgmichel May 5, 2023
bd734b1
- `code` must be able to oog
tgmichel May 11, 2023
0a22130
Merge branch 'master' into tgm-record-external-costs
tgmichel May 12, 2023
1a1a47f
`is_empty` accounting
tgmichel May 12, 2023
a63f1d2
fix build
tgmichel May 12, 2023
e59eb9c
`SSTORE` must record account storage proof size
tgmichel May 12, 2023
91a5259
suggestion: move weight_limit checks
tgmichel May 16, 2023
3915652
editorconfig
tgmichel May 16, 2023
d09fc5b
fmt
tgmichel May 16, 2023
e9df020
rename `transaction_len` to `proof_size_base_cost` in runner
tgmichel May 16, 2023
a8fd45e
move `proof_size_base_cost` to validation primitive
tgmichel May 16, 2023
caefbec
gas limit saturated conversion
tgmichel May 24, 2023
df032b5
remove transaction proof size check outside validation
tgmichel May 24, 2023
e53da69
Merge branch 'master' into tgm-record-external-costs
tgmichel May 25, 2023
76c2f6c
pin evm
tgmichel May 25, 2023
6f6827a
pin evm+
tgmichel May 25, 2023
0154cc9
fix todos
tgmichel May 25, 2023
881f690
fix build
tgmichel May 25, 2023
eddef23
scope of already recorded codes and storages must be per transaction
tgmichel Jun 9, 2023
d6d4765
pin evm + implement new `record_external_operation`
tgmichel Jun 14, 2023
12fbeee
fix runtime api versioning + legacy `ExecutionInfo` handling
tgmichel Jun 19, 2023
8a968e9
Merge branch 'master' into tgm-record-external-costs
tgmichel Jun 19, 2023
da32bba
editorconfig
tgmichel Jun 19, 2023
5ba5fcc
cargo fmt
tgmichel Jun 19, 2023
6d3bfb3
clippy
tgmichel Jun 19, 2023
ebd1486
suggestion remove `evm-with-weight-limit` feature
tgmichel Jun 21, 2023
7198ca4
fmt
tgmichel Jun 21, 2023
d95b7ed
update comment
tgmichel Jun 21, 2023
d9f12de
update tests for additional `AccountBasicRead` in the evm
tgmichel Jun 21, 2023
074711b
update evm pin
tgmichel Jun 21, 2023
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
4 changes: 3 additions & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ jobs:
repo-token: ${{ secrets.GITHUB_TOKEN }}
- name: Run tests
run: cargo test --locked --verbose --all
- name: Run tests (evm-with-weight-limit)
run: cargo test --locked --verbose --all --features evm-with-weight-limit

integration:
name: 'Run integration tests'
Expand Down Expand Up @@ -89,4 +91,4 @@ jobs:
- name: Rustfmt
run: cargo fmt --all -- --check
- name: Clippy
run: cargo clippy --all --features runtime-benchmarks,try-runtime -- -D warnings
run: cargo clippy --all --features runtime-benchmarks,try-runtime,evm-with-weight-limit -- -D warnings
21 changes: 13 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ bn = { package = "substrate-bn", version = "0.6", default-features = false }
environmental = { version = "1.1.3", default-features = false }
ethereum = { version = "0.14.0", default-features = false }
ethereum-types = { version = "0.14.1", default-features = false }
evm = { version = "0.37.0", default-features = false }
evm = { git = "https://github.com/purestake/evm", branch = "tgm-record-external-cost", default-features = false }
tgmichel marked this conversation as resolved.
Show resolved Hide resolved
impl-serde = { version = "0.4.0", default-features = false }
jsonrpsee = "0.16.2"
kvdb-rocksdb = "0.17.0"
Expand Down Expand Up @@ -76,6 +76,7 @@ sc-transaction-pool = { version = "4.0.0-dev", git = "https://github.com/parityt
sc-transaction-pool-api = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", branch = "master" }
# Substrate Primitive
sp-api = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-arithmetic = { version = "6.0.0", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-block-builder = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-blockchain = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", branch = "master" }
sp-consensus = { version = "0.10.0-dev", git = "https://github.com/paritytech/substrate", branch = "master" }
Expand Down
1 change: 1 addition & 0 deletions frame/ethereum/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,4 @@ try-runtime = [
"pallet-evm/try-runtime",
]
forbid-evm-reentrancy = ["pallet-evm/forbid-evm-reentrancy"]
evm-with-weight-limit = ["evm/with-substrate", "pallet-evm/evm-with-weight-limit"]
96 changes: 86 additions & 10 deletions frame/ethereum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,24 @@ where
len: usize,
) -> Option<TransactionValidity> {
if let Call::transact { transaction } = self {
// Validate submitted gas limit to weight conversion
let gas_limit = match transaction {
Transaction::Legacy(t) => t.gas_limit,
Transaction::EIP2930(t) => t.gas_limit,
Transaction::EIP1559(t) => t.gas_limit,
};
let without_base_extrinsic_weight = true;
let computed_weight_limit = T::GasWeightMapping::gas_to_weight(
gas_limit.unique_saturated_into(),
without_base_extrinsic_weight,
);
if computed_weight_limit != dispatch_info.weight {
tgmichel marked this conversation as resolved.
Show resolved Hide resolved
return Some(Err(TransactionValidityError::Invalid(
InvalidTransaction::Custom(255),
)));
}

// CheckWeight
if let Err(e) = CheckWeight::<T>::do_validate(dispatch_info, len) {
return Some(Err(e));
}
Expand Down Expand Up @@ -241,7 +259,7 @@ pub mod pallet {
Self::validate_transaction_in_block(source, &transaction).expect(
"pre-block transaction verification failed; the block cannot be built",
);
let r = Self::apply_validated_transaction(source, transaction)
let r = Self::apply_validated_transaction(source, transaction, None)
.expect("pre-block apply transaction failed; the block cannot be built");

weight = weight.saturating_add(r.actual_weight.unwrap_or_default());
Expand Down Expand Up @@ -290,7 +308,32 @@ pub mod pallet {
"pre log already exists; block is invalid",
);

Self::apply_validated_transaction(source, transaction)
let gas_limit: u64 = match transaction {
Transaction::Legacy(ref t) => t.gas_limit.unique_saturated_into(),
Transaction::EIP2930(ref t) => t.gas_limit.unique_saturated_into(),
Transaction::EIP1559(ref t) => t.gas_limit.unique_saturated_into(),
};

let weight_limit =
match <T as pallet_evm::Config>::GasWeightMapping::gas_to_weight(gas_limit, true) {
weight_limit if weight_limit.proof_size() > 0 => {
// Try to subtract the encoded extrinsic from the Weight proof_size limit or fail
// Validate the weight limit can afford recording transaction len
let _ = weight_limit
.proof_size()
.checked_sub(Self::transaction_len(&transaction))
.ok_or(DispatchErrorWithPostInfo {
post_info: PostDispatchInfo {
actual_weight: None,
pays_fee: Pays::No,
},
error: sp_runtime::DispatchError::Exhausted,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put the verification into the validate_transaction_in_pool? In this way, you do not need to add another weight_limit input params to the apply_validated_transaction, we can handle the checked_sub(transaction_len) in the execute() function together with transaction_len.

let transaction_len = if weight_limit.is_some() {
			Some(Self::transaction_len(transaction))
		} else {
			None
		};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @boundless-forest , I applied your suggestion although I'm not entirely sure if it will be reverted before merging.

@librelois removing this param means we rely 100% on the ratio-based conversion from gas_limit to pov_size_limit. Do we want to be able to keep the possibility for overriding this weight_info? I'm thinking in Xcm->Evm for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to be able to keep the possibility for overriding this weight_info?

I think so, frontier should be generic and future proof

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@librelois I see what you mean but I have mixed feelings on this. On one hand I agree leaving a more open api design is easier for now, but in the other I don't see why we should: I cannot think of a case where we need the WeightInfo override. Also there is nothing preventing we change it in the future if we end up needing it.

I think we should leave @boundless-forest suggestion by now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, sound's reasonable

})?;
Some(weight_limit)
}
_ => None,
};
Self::apply_validated_transaction(source, transaction, weight_limit)
}
}

Expand Down Expand Up @@ -353,6 +396,16 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
fn transaction_len(transaction: &Transaction) -> u64 {
transaction
.encode()
.len()
// pallet index
.saturating_add(1)
// call index
.saturating_add(1) as u64
}

fn recover_signer(transaction: &Transaction) -> Option<H160> {
let mut sig = [0u8; 65];
let mut msg = [0u8; 32];
Expand Down Expand Up @@ -535,14 +588,15 @@ impl<T: Config> Pallet<T> {
fn apply_validated_transaction(
source: H160,
transaction: Transaction,
weight_limit: Option<Weight>,
) -> DispatchResultWithPostInfo {
let (to, _, info) = Self::execute(source, &transaction, None)?;
let (to, _, info) = Self::execute(source, &transaction, None, weight_limit)?;

let pending = Pending::<T>::get();
let transaction_hash = transaction.hash();
let transaction_index = pending.len() as u32;

let (reason, status, used_gas, dest, extra_data) = match info {
let (reason, status, weight_info, used_gas, dest, extra_data) = match info {
CallOrCreateInfo::Call(info) => (
info.exit_reason.clone(),
TransactionStatus {
Expand All @@ -558,6 +612,7 @@ impl<T: Config> Pallet<T> {
bloom
},
},
info.weight_info,
info.used_gas,
to,
match info.exit_reason {
Expand Down Expand Up @@ -601,6 +656,7 @@ impl<T: Config> Pallet<T> {
bloom
},
},
info.weight_info,
info.used_gas,
Some(info.value),
Vec::new(),
Expand Down Expand Up @@ -656,10 +712,16 @@ impl<T: Config> Pallet<T> {
});

Ok(PostDispatchInfo {
actual_weight: Some(T::GasWeightMapping::gas_to_weight(
used_gas.unique_saturated_into(),
true,
)),
actual_weight: {
let mut gas_to_weight =
T::GasWeightMapping::gas_to_weight(used_gas.unique_saturated_into(), true);
if let Some(weight_info) = weight_info {
if let Some(proof_size_usage) = weight_info.proof_size_usage {
*gas_to_weight.proof_size_mut() = proof_size_usage;
tgmichel marked this conversation as resolved.
Show resolved Hide resolved
}
}
Some(gas_to_weight)
},
pays_fee: Pays::No,
})
}
Expand All @@ -674,6 +736,7 @@ impl<T: Config> Pallet<T> {
from: H160,
transaction: &Transaction,
config: Option<evm::Config>,
weight_limit: Option<Weight>,
) -> Result<
(Option<H160>, Option<H160>, CallOrCreateInfo),
DispatchErrorWithPostInfo<PostDispatchInfo>,
Expand Down Expand Up @@ -740,6 +803,11 @@ impl<T: Config> Pallet<T> {

let is_transactional = true;
let validate = false;
let transaction_len = if weight_limit.is_some() {
Some(Self::transaction_len(transaction))
} else {
None
};
match action {
ethereum::TransactionAction::Call(target) => {
let res = match T::Runner::call(
Expand All @@ -754,6 +822,8 @@ impl<T: Config> Pallet<T> {
access_list,
is_transactional,
validate,
weight_limit,
transaction_len,
config.as_ref().unwrap_or_else(|| T::config()),
) {
Ok(res) => res,
Expand Down Expand Up @@ -782,6 +852,8 @@ impl<T: Config> Pallet<T> {
access_list,
is_transactional,
validate,
weight_limit,
transaction_len,
config.as_ref().unwrap_or_else(|| T::config()),
) {
Ok(res) => res,
Expand Down Expand Up @@ -900,8 +972,12 @@ impl<T: Config> Pallet<T> {

pub struct ValidatedTransaction<T>(PhantomData<T>);
impl<T: Config> ValidatedTransactionT for ValidatedTransaction<T> {
fn apply(source: H160, transaction: Transaction) -> DispatchResultWithPostInfo {
Pallet::<T>::apply_validated_transaction(source, transaction)
fn apply(
source: H160,
transaction: Transaction,
weight_limit: Option<Weight>,
) -> DispatchResultWithPostInfo {
Pallet::<T>::apply_validated_transaction(source, transaction, weight_limit)
}
}

Expand Down
2 changes: 1 addition & 1 deletion frame/ethereum/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ parameter_types! {
pub const TransactionByteFee: u64 = 1;
pub const ChainId: u64 = 42;
pub const EVMModuleId: PalletId = PalletId(*b"py/evmpa");
pub const BlockGasLimit: U256 = U256::MAX;
pub BlockGasLimit: U256 = U256::from(150_000_000);
pub const WeightPerGas: Weight = Weight::from_ref_time(20_000);
}

Expand Down
Loading