ExecutedBlock cleanup #7991
ExecutedBlock cleanup #7991
Conversation
debris
added
A0-pleasereview 🤓
| // Bestow block reward | ||
| let res = fields.state.add_balance(&receiver, &reward, CleanupMode::NoEmpty) | ||
| .map_err(::error::Error::from) | ||
| .and_then(|_| fields.state.commit()); |
There was a problem hiding this comment.
issue 1 -> bestow_block_reward is called from on_close_block and it does state.commit(). It shouldn't. What's more this function is called in a loop, so it's possible that there were multiple commits made in a single on_close_block.
| }); | ||
|
|
||
| if let Err(ref e) = res { | ||
| warn!("Encountered error on bestowing reward: {}", e); |
There was a problem hiding this comment.
issue 2 -> error is silently ignored
ethcore/src/block.rs
Outdated
| @@ -84,6 +84,38 @@ impl Decodable for Block { | |||
| } | |||
| } | |||
|
|
|||
| /// Container for block traces. | |||
| #[derive(Clone)] | |||
| pub enum Tracing { | |||
There was a problem hiding this comment.
Don't feel like it's a right place to store this structure.
| let traces: Vec<FlatTransactionTraces> = traces.into_iter() | ||
| .map(Into::into) | ||
| .collect(); | ||
| let traces = block.traces().clone().drain(); |
There was a problem hiding this comment.
Seems like unnecessary clone, since block is discarded anyway.
There was a problem hiding this comment.
totally agree!
But for now, we can't omit clone(), cause block is generic over IsBlock trait, and IsBlock has only fn traces(&self) method.
There was a problem hiding this comment.
that's why I wrote, that in next pr
I'll try to reduce number of clones we make during block verification ;)
If you are talking about #7038, it doesn't modify |
| ::engines::common::bestow_block_reward(block, self.block_reward, empty_step.author()?)?; | ||
| let author = empty_step.author()?; | ||
| self.machine.add_balance(block, &author, &self.block_reward)?; | ||
| self.machine.note_rewards(block, &[(author, self.block_reward)], &[])?; |
There was a problem hiding this comment.
I think this would be more efficient if we had only one call to note_rewards.
debris
added
A4-gotissues 💥
debris
added
A0-pleasereview 🤓
changes:
BlockRefandBlockRefMutstructureTracingwhich represents underlaying data better thanOption<Vec<Vec<FlatTrace>>>bestow_block_rewardfunction which, I believe, contained bugs. Use machine insteadI next prs
I will get rid ofI'll try to reduce number of clones we make during block verificationBlocksRefstructure and