Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

[WIP] Hybrid Casper FFG Implementation #8654

Closed
wants to merge 75 commits into from
Closed

[WIP] Hybrid Casper FFG Implementation #8654

wants to merge 75 commits into from

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented May 18, 2018

cc #7162, djrtwo/EIPs#5. This is a work-in-progress. Do not merge. Reviews/comments/grumbles are highly appreciated.

This implements EIP-1011. Currently we have initialization/deployment, vote transactions validation/sorting, fork choice and finalization implemented. Command line options are not added and there's no block reward issuance modifications for ease of testing. EIP-1011's current specification is not compatible with the old testnet, so only local testing is possible.

Current known issues:

  • Snapshot, warp sync, light client won't work with Casper right now.
  • Miner's vote transaction selection is really inefficient. It probably only works for local vote transactions now. We might need a new struct TransactionPicker which picks the next transaction to be included in a block.
  • Miner's block reopening won't work with Casper. Because of the introduction of vote transaction ordering, when a new transaction comes, we cannot just reopen the current sealing block, revert to the last transaction's executed state, execute the new transactions and then close the block. Whenever we need to do this, it seems that we need to re-execute at least all of the vote transactions. @djrtwo do you have any suggestions how this might be fixed?

@sorpaas sorpaas added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels May 18, 2018
@5chdn
Copy link
Contributor

5chdn commented May 18, 2018

EIP-1011's current specification is not compatible with the old testnet, so only local testing is possible.

Well, let's start an EIP-1011 testnet then? Happy to set this up. Other clients working on that spec can join at any point.

@5chdn 5chdn added this to the 1.12 milestone May 18, 2018
@sorpaas
Copy link
Collaborator Author

sorpaas commented May 18, 2018

@5chdn Need some more polishing and testing, and in particular, we need to have a validator implementation (probably just an external one through JSONRPC endpoints). Otherwise we cannot interact with the Casper contract. Many of the current parameters used here will also likely need to be changed because EIP1011 has not been finalized. But anyway, no matter testing locally, privately or publicly, I think having a testnet is important, and I'll definitely try to get it done soon.

Besides, I think @djrtwo, and in particular, the py-evm group is planning to launch another new testnet soon. So we may also consider to join that.

@djrtwo
Copy link

djrtwo commented May 18, 2018

@5chdn Yeah, the testnet will be pretty boring without validators 😝. There was one written in pyethapp that is now deprecated. Harmony was also implementing one, but it was for the pre-EIP spec.

Vyper just released some updates that we needed on the python side of things to get the EIP implementation going in py-evm. Trying to get out any implementation next week and start coordinating a multi-client testnet from there.

@sorpaas You should theoretically be able to open up the block, add a nonvote tx and not re-run the votes. Casper vote txs can affect block transaction execution, but normal block transactions cannot affect casper votes. Essentially the post block state is all the normal transactions + limited state modification from the votes (state that the normal txs can't touch). The limited state modification from the votes will be the same regardless of what normal transactions were placed in front of it. I'm not sure how the internals of parity work, but you should be able apply with vote state mod to any-prestate without re-running.

@sorpaas
Copy link
Collaborator Author

sorpaas commented May 29, 2018

A note on transaction queue: currently we mark vote transactions as service transactions, so propagation should have no issue, and we can implement DoS methods with this. However, to "fully fill" a block with vote transactions, this may require to extend MAX_SKIPPED_TRANSACTIONS to a much larger value. So eventually this needs to be replaced by two transaction queues.

We can use json config to customize them now.
@sorpaas
Copy link
Collaborator Author

sorpaas commented May 30, 2018

cc @5chdn

Just spinned up a testnet using this branch so that we can test out whether the networking and txpool propagation works alright, before we get the testnet from @djrtwo. 😃 You can also use this to test any validator implementation against Casper revision 0.2.0. Please PM me if you need any testnet tokens. :)

Specifications:

  • Use this file and binary from this branch casper to connect to the network.
  • The SimpleCasper contract is compiled using master Vyper with default settings at tag v0.2.0, and is at address 0x40.
  • The PurityChecker contract is at address 0x41.
  • The MsgHasher contract is at address 0x42.
  • The RlpDecoder contract is at address 0x43.
  • Casper transition happened at block 50, and the warm up period is 50.
  • Other parameters are the same as in EIP1011, except the vote transaction ordering requirement is waived, which according to @djrtwo, will be reflected in a future version of the EIP1011 spec.
  • Block reward reduction is not handled for easy testing.
  • EIP155 is 0x1ee7.

@ghost

This comment has been minimized.


/// Casper-specific fork choice.
pub fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> ForkChoice {
let new_metadata: HybridCasperMetadata = new.metadata().map(|d| rlp::decode(d).expect("Metadata is only set by serializing CasperMetadata struct; deserailzling CasperMetadata RLP always succeeds; qed")).unwrap_or(Default::default());
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: "deserailzling" (also several times below)


let mut metadata: HybridCasperMetadata = block.metadata().map(|d| rlp::decode(d).expect("Metadata is only set by serializing CasperMetadata struct; deserailzling CasperMetadata RLP always succeeds; qed")).unwrap_or(Default::default());
metadata.vote_gas_used = receipt.gas_used;
receipt.gas_used = block.receipts().last().map(|r| r.gas_used).unwrap_or(U256::zero());
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe unwrap_or_else(U256::zero) would be better: It avoids creating the zero U256 if there was a receipt.


/// Casper-specific fork choice.
pub fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> ForkChoice {
let new_metadata: HybridCasperMetadata = new.metadata().map(|d| rlp::decode(d).expect("Metadata is only set by serializing CasperMetadata struct; deserailzling CasperMetadata RLP always succeeds; qed")).unwrap_or(Default::default());
Copy link
Contributor

Choose a reason for hiding this comment

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

unwrap_or_else(Default::default) would avoid unnecessarily instantiating HybridCasperMetadata (also a few times below).

@5chdn 5chdn added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Jun 18, 2018
@5chdn
Copy link
Contributor

5chdn commented Jun 18, 2018

Putting this on ice as per dev call last week. Are you continuing work on this branch or shall we close it?

@sorpaas
Copy link
Collaborator Author

sorpaas commented Jun 18, 2018

No. Let's close this for now.
I think the beacon chain design would require a completely different set of code.

@sorpaas sorpaas closed this Jun 18, 2018
@5chdn
Copy link
Contributor

5chdn commented Jun 18, 2018

😢

@sorpaas sorpaas mentioned this pull request Jul 19, 2018
@5chdn 5chdn deleted the casper branch November 1, 2018 10:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants