Skip to content
This repository has been archived by the owner on Mar 13, 2023. It is now read-only.

Refactor Ethereum Primitives #708

Merged
merged 23 commits into from
Jul 27, 2021
Merged

Refactor Ethereum Primitives #708

merged 23 commits into from
Jul 27, 2021

Conversation

AurevoirXavier
Copy link
Member

@AurevoirXavier AurevoirXavier commented Jul 12, 2021

  • Drop duplicate primitive library.
     // e.g.
     // `H64`, `H256` from different library
     use primitive_types::H64;
     use ethereum_types::H256;
  • Clean unnecessary dependencies speed up the compiling.
  • Generic library with multi-features control, easy to use for side projects such as bridger, shadow, etc.
  • Better error handling.
  • Remove EthereumHeader::from_str_unchecked #271
    • New serializer/deserializer accept all Infura API like header spec. With this feature added, shadow does not need to handle the DarwiniaHeader spec. And we could finish the serialize/deserialize in bridger part with library ethereum-primitives. cc @xiaoch05
  • Ethereum London Hardfork support #702
    • Support EIP-1559 #701
      • On-chain storage checks. There is a new field named base_fee_per_gas, which type is an Option. Maybe it's compatible with scale-codec.
      • Migration to new header spec.
        • New header spec.
        • Impl RLP encoder/decoder for new header.
        • Reset Relay::ConfirmedHeaderParcels.
        • Reset RelayerGame::Affirmations.
    • Support EIP-3554.
      • Add difficulty bomb delay.
  • Maybe Refactor ethereum-primitives & chain-bsc #655.

@AurevoirXavier AurevoirXavier added Bridge C-Dependency [Component] Something about dependency N-Pangolin [Network] Pangolin U-Test Improvement [Uncategorized] Test improvement labels Jul 12, 2021
@AurevoirXavier AurevoirXavier marked this pull request as draft July 13, 2021 03:01
@AurevoirXavier AurevoirXavier force-pushed the xavier-ethereum-primitives branch 3 times, most recently from 647d6aa to 06d5214 Compare July 16, 2021 02:47
@hackfisher
Copy link
Contributor

hackfisher commented Jul 18, 2021

Ethereum and BSC may use different version of primitives, not reasonable to merge and unify them, any tiny upgrade from chain side changing the primitives could break this.

@AurevoirXavier AurevoirXavier linked an issue Jul 21, 2021 that may be closed by this pull request
2 tasks
@AurevoirXavier
Copy link
Member Author

AurevoirXavier commented Jul 22, 2021

panicked at 'called `Result::unwrap()` on an `Err` value: Error { cause: Some(Error { cause: Some(Error { cause: None, desc: "Not enough data to fill buffer" }), desc: "Could not decode variant byte for `Option`" }), desc: "Could not decode `Header::hash`" }'

EIP-1559 introduces a new field in the header base_fee_per_gas, which will break the scale-type encoded storage.

For us, we could clear the ConfirmedHeaderParcels storage then apply the new type. Or define a new storage name. (e.g. EIP1559HeaderParcels).

cc @hackfisher

@hackfisher
Copy link
Contributor

panicked at 'called `Result::unwrap()` on an `Err` value: Error { cause: Some(Error { cause: Some(Error { cause: None, desc: "Not enough data to fill buffer" }), desc: "Could not decode variant byte for `Option`" }), desc: "Could not decode `Header::hash`" }'

EIP-1559 introduces a new field in the header base_fee_per_gas, which will break the scale-type encoded storage.

For us, we could clear the ConfirmedHeaderParcels storage then apply the new type. Or define a new storage name. (e.g. EIP1559HeaderParcels).

cc @hackfisher

Prefer clear & migrate the ConfirmedHeaderParcels storage then apply the new type.

@AurevoirXavier
Copy link
Member Author

The genesis's MMR root is wrong.
Need shadow to provide the right one.

cc @xiaoch05

This was linked to issues Jul 26, 2021
@AurevoirXavier
Copy link
Member Author

AurevoirXavier commented Jul 27, 2021

@AurevoirXavier AurevoirXavier marked this pull request as ready for review July 27, 2021 02:48
@hackfisher hackfisher merged commit a09bec8 into master Jul 27, 2021
@hackfisher hackfisher deleted the xavier-ethereum-primitives branch July 27, 2021 02:50
@@ -105,6 +110,7 @@ impl EthashPartial {
m.insert(4370000, 3000000);
m.insert(7280000, 2000000);
m.insert(0x8c6180, 0x3d0900);
m.insert(0xc3d0e8, 0xaae60);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot where I got these values.
Should we update this to the latest openethereum code in the next runtime upgrade?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I double checked with geth, openethereum's code should be right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should this be included in darwinia-v0.11.7?
I'll fire a PR right 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, darwinia-v0.11.7 branch is a good place, and don't forget we also need it in master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C-Dependency [Component] Something about dependency N-Pangolin [Network] Pangolin U-Test Improvement [Uncategorized] Test improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ethereum London Hardfork support Support EIP-1559 Remove EthereumHeader::from_str_unchecked
3 participants