implement eth_call state override

[nbaztec]

Implements call state override for eth_call as an optional parameter to be passed to override the state. Similar to geth.

[sorpaas]

Tests fail.

[nbaztec]

Tests fail.

Odd I cannot seem to reproduce that failure locally. I tried several times:

$ cd template/node
$ cargo build --release --locked --verbose --features rpc-binary-search-estimate
...
$ cd ts-tests
$ npm test

and it is consistently passing.

Is there anything else that might involve the template node being built differently?

The test that fails is:

1) Frontier RPC (StateOverride)
       should have a sender balance of 150 with state override:

which pertains to the storage override for System.Account pallet via runtime_storage_override in EthDeps, so my guess would be that something doesn't work there effectively, but I can't see why as it's working locally just fine.

EDIT: Seems for some reason the address mapping on CI is IdentityAddressMapping but locally I get HashedAddressMapping

[sorpaas]

I see you already figured out the CI issue -- you probably didn't delete the old local runtime folder.

[sorpaas]

Evm should be all capitalized EVM as it is elsewhere. Alternatively, you can also just name it RuntimeAddressMapping since the EVM part is non-ambiguous. Same for the trait below.

In addition, make this trait non-implementable by having a generic impl, because we already have the AddressMapping trait.

impl<T: AsRef<[u8]>, U: pallet_evm::AddressMapping<T>> RuntimeAddressMapping for U {
  fn into_account_id_bytes(address: H160) -> Vec<u8> {
    U::into_account_id(address).as_ref().to_vec()
  }
}

You can probably also just reuse AddressMapping<Vec<u8>>, but I'm not sure if there's specialization issues.

[nbaztec]

Yeah I thought about using AddressMapping<Vec<u8>> but since there's an impl impl<T: From<H160>> already in causes ambiguous implementation since From<H160> can implemented for Vec<u8> as well. It could be solved by adding the fn into_account_id_bytes to the trait AddressMapping and may be reused while computing the account_id

impl<H: Hasher<Out = H256>> AddressMapping<AccountId32> for HashedAddressMapping<H> {
	fn into_account_id(address: H160) -> AccountId32 {
		AccountId32::from_slice(&Self::into_account_id_bytes(address));
	}

	fn into_account_id_bytes(address: H160) -> Vec<u8> {
		let mut data = [0u8; 24];
		data[0..4].copy_from_slice(b"evm:");
		data[4..24].copy_from_slice(&address[..]);
		let hash = H::hash(&data);

		Into::<[u8; 32]>::into(hash).to_owned()

	}
}

thoughts?

[sorpaas]

If you can make into_account_id_bytes a default impl, then it should work. However, this will require adding AsRef<[u8]> constraint to T, which I think is okay.

[nbaztec]

Seems in the pallet-evm Config we pass AccountId to AddressMapping which doesn't have the AsRef<[u8]> trait bound

[sorpaas]

I think this is the time we should group EGA together with M into an RPC config trait (because they can not be implied).

[nbaztec]

Could you perhaps elaborate how this trait might look like?

[sorpaas]

Basically this old PR #736

We didn't went with all the type parameters because most of them can be inferred by the compiler. In the past we only had a single one EGA which belongs to Config. The new type parameter we have in this PR also belongs to Config.

Something like this:

trait EthConfig {
   type EstimateGasAdapter;
   type RuntimeAddressMapping;
}

Then in Eth put the type parameter Config: EthConfig.

[nbaztec]

@sorpaas could you please take a look if this is good now? Thanks.

[sorpaas]

Suggested change

	pub fn with_eth_config<EC2: EthConfig<B, C>>(self) -> Eth<B, C, P, CT, BE, H, A, EC2> {
	pub fn replace_config<EC2: EthConfig<B, C>>(self) -> Eth<B, C, P, CT, BE, H, A, EC2> {

with_eth_config sounds like a constructor, which this isn't. Our previous with_estimate_gas_adapter had the same problem.

[sorpaas]

Remove this and the () impl for EthConfig?

Or alternatively, make this "no-op" set_overlay_changes. into_account_id_bytes should be able to have a default impl for AsRef<[u8]>. This may be preferred as it impact existing users who don't care about overlay changes the least.

[nbaztec]

Yeah we could go with no-op the is_enabled can stay false to signal an incoming call with balance and nonce overrides with an error that it's unsupported.

[nbaztec]

into_account_id_bytes is tightly coupled with RuntimeStorageOverride now so it should be sufficient to return the default value within the () impl.