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

Add Ethereum components from Frontier #9

Merged
merged 35 commits into from
Jul 11, 2023
Merged

Conversation

notlesh
Copy link
Contributor

@notlesh notlesh commented Jun 7, 2023

This PR adds Ethereum compatibility components from Frontier. Solves #4.

TODO:

  • Add EVM and Ethereum to runtime
  • Implement AccountMapping scheme
  • Add very basic RPC client
  • Implement pallet_evm::FindAuthor
  • Add tests, examples, documentation, etc. as needed
  • Precompiles
  • Move ManagedAddressMapping to runtime/src with other pallets
  • Merge main

@notlesh notlesh mentioned this pull request Jun 12, 2023
@notlesh

This comment was marked as resolved.

@JoshOrndorff

This comment was marked as resolved.

@notlesh

This comment was marked as resolved.

@fbielejec

This comment was marked as resolved.

@notlesh

This comment was marked as outdated.

@JoshOrndorff
Copy link
Collaborator

I'm testing this manually trying to get it merged soon. I want to be able to deploy and execute some contract in the evm, and I can't yet do that.

I'm trying with remix and also with polkadot js apps. Trying the node in instant seal and pow. It seems like I'm having fee related issues. Is there some particular gas price you have in mind, or other tips?

@JoshOrndorff
Copy link
Collaborator

JoshOrndorff commented Jun 24, 2023

I've messed with the address mapping a little bit. One bug I noticed is that both Alice and Bob can map the same ethereum H160 address, let's call it Gerald. This is trouble because if Gerald sends an eth transaction to pallet ethereum, only one of Alice or Bob will be charged fees.

It might be nice to make a similar check to the one you make for Native addresses first.

On a related note, I've got a start installing unified accounts which I think would be really nice in #20. What do you think about using that and eliminating the manual mapping?

One thing we have to check is whether tolls that were built for pallet contracts are smart enough to support that.

@JoshOrndorff
Copy link
Collaborator

I've disabled fees, and I'm now able to deploy and call a contract using polkadot js. That's a good start! Now to Metamask.

I can see my balance (with a funny number of decimals). But when I try to send tokens the transactions fail.

@JoshOrndorff JoshOrndorff mentioned this pull request Jun 24, 2023
Comment on lines +473 to +497
#[derive(Clone)]
pub struct TransactionConverter;

impl fp_rpc::ConvertTransaction<UncheckedExtrinsic> for TransactionConverter {
fn convert_transaction(&self, transaction: pallet_ethereum::Transaction) -> UncheckedExtrinsic {
UncheckedExtrinsic::new_unsigned(
pallet_ethereum::Call::<Runtime>::transact { transaction }.into(),
)
}
}

impl fp_rpc::ConvertTransaction<opaque::UncheckedExtrinsic> for TransactionConverter {
fn convert_transaction(
&self,
transaction: pallet_ethereum::Transaction,
) -> opaque::UncheckedExtrinsic {
let extrinsic = UncheckedExtrinsic::new_unsigned(
pallet_ethereum::Call::<Runtime>::transact { transaction }.into(),
);
let encoded = extrinsic.encode();
opaque::UncheckedExtrinsic::decode(&mut &encoded[..])
.expect("Encoded extrinsic is always valid")
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this is no longer necessary. See issue polkadot-evm/frontier#508 and PR polkadot-evm/frontier#559 for context.

@notlesh notlesh mentioned this pull request Jul 6, 2023
@JoshOrndorff JoshOrndorff merged commit 025dec5 into main Jul 11, 2023
@JoshOrndorff JoshOrndorff deleted the notlesh-add-frontier branch July 11, 2023 06:33
@JoshOrndorff JoshOrndorff mentioned this pull request Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants