Rewrite runBlock internals with async/await #494
Conversation
| * @type {Object} | ||
| * @property {Block} block emits the block that is about to be processed | ||
| */ | ||
| await this._emit('beforeBlock', opts.block) |
There was a problem hiding this comment.
Ok (checking per async task).
| */ | ||
| self.emit('afterBlock', result, cb) | ||
| // Checkpoint state | ||
| await state.checkpoint() |
There was a problem hiding this comment.
setStateRoot, checkpointState logic ok, just for reference: order of validateBlock has been changed respectively moved further down the line.
| // Pay ommers and miners | ||
| await assignBlockRewards.bind(this)(block) | ||
| return txResults | ||
| } |
There was a problem hiding this comment.
Moved validateBlock, processTransactions and payOmmersAndMiners here, introduced hierarchical call structure for applyBlock and applyTransactions, good.
| let gasUsed = new BN(0) | ||
| const receiptTrie = new Trie() | ||
| const receipts = [] | ||
| const txResults = [] |
There was a problem hiding this comment.
Like this more local and targeted initialization of variables.
| cb(err) | ||
| } | ||
| return | ||
| const txReceipt = { |
There was a problem hiding this comment.
Some cleanup here on receipt creation, good.
| self.stateManager.getStateRoot(function (err, stateRoot) { | ||
| if (err) return cb(err) | ||
| return { bloom, gasUsed, receiptRoot: receiptTrie.root, receipts, results: txResults } | ||
| } |
There was a problem hiding this comment.
For reference: inlined the result processing from parseTxResult.
| const totalNiblingReward = niblingReward.muln(ommers.length) | ||
| const reward = minerReward.add(totalNiblingReward) | ||
| return reward | ||
| } |
There was a problem hiding this comment.
Some cleanup on miner/ommer (side note: we should rename to "uncle", this confused me for quite some time) code, looks good.
|
Additional note: we should add some more API to this sometime. |
This PR:
runBlockinternals with promises and async/await syntaxrunBlockand (similar torunTx) is the first iteration.Some side note: Currently the types we have are very heavy and serve multiple purposes. E.g.
ethereumjs-blockcontains RLP serialization/deserialization of blocks, db interactions and also validation and other logic. I think we should break them and have separate types for each of these purposes. One type for RLP serialization, one for storing/fetching from db (which uses RLPBlock), one for consensus logic. The same applies toBlockchainandTx. Not sure yet about details, but thought I'd ask for feedback on this idea. One other benefit for this separation is that we could then have a base class for the consensus logic part of the type, and have a child class for each fork for that type, e.g.PetersburgBlockwhich contains the logic for blocks afterPetersburg.(To be merged into #479)