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

EIP-98: Optional transaction state root #4296

Merged
merged 3 commits into from
Jan 25, 2017
Merged

EIP-98: Optional transaction state root #4296

merged 3 commits into from
Jan 25, 2017

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Jan 25, 2017

This PR removes intermediate trie commits when EIP-98 is enabled. Actual transaction parallelization is to follow.

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 25, 2017
@@ -375,6 +375,9 @@ impl<'x> OpenBlock<'x> {
let unclosed_state = s.block.state.clone();

s.engine.on_close_block(&mut s.block);
if let Err(e) = s.block.state.commit() {
warn!("Encountered error on state commit: {}", e);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't that error be propagated upstream?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is how commit error are handles in the rest of the codebase.

@@ -65,6 +67,7 @@ impl From<ethjson::spec::Params> for CommonParams {
subprotocol_name: p.subprotocol_name.unwrap_or_else(|| "eth".to_owned()),
min_gas_limit: p.min_gas_limit.into(),
fork_block: if let (Some(n), Some(h)) = (p.fork_block, p.fork_hash) { Some((n.into(), h.into())) } else { None },
eip98_transition: p.eip98_transition.map_or(0, Into::into),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't that enable eip98 automatically for spec files where that prop is not defined (private chains?)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that is the intention

@@ -519,8 +519,14 @@ impl State {

// TODO uncomment once to_pod() works correctly.
// trace!("Applied transaction. Diff:\n{}\n", state_diff::diff_pod(&old, &self.to_pod()));
self.commit()?;
let receipt = Receipt::new(self.root().clone(), e.cumulative_gas_used, e.logs);
let state_root = match env_info.number < engine.params().eip98_transition {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using if-else here would a bit more idiomatic and suprisingly shorter then match.

@tomusdrw tomusdrw added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 25, 2017
@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jan 25, 2017
@tomusdrw tomusdrw added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 25, 2017
@arkpar arkpar changed the title EIP-98: Optional receipt state root EIP-98: Optional trabsaction state root Jan 25, 2017
@arkpar arkpar changed the title EIP-98: Optional trabsaction state root EIP-98: Optional transaction state root Jan 25, 2017
@arkpar arkpar added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jan 25, 2017
@gavofyork gavofyork self-requested a review January 25, 2017 10:01
@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Jan 25, 2017
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 25, 2017
@gavofyork gavofyork merged commit c012dfc into master Jan 25, 2017
@gavofyork gavofyork deleted the eip98 branch January 25, 2017 19:22
@arkpar arkpar mentioned this pull request Mar 9, 2017
12 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants