Minor post-merge cleanups #3945
Conversation
#3944 The use of nested `awaitWithRetries` calls would have resulted in an unexpected number of retries (3x3). We now use regular `await` in outer layer to avoid the problem. #3943 The new code has an invariant that the `headMerkleizer` field in the `Eth1Chain` is always kept in sync with the blocks stored in the chain. This invariant is now enforced better by doing the necessary merkleizer updates in the `Eth1Chain.addBlock` function.
| ## This function will return true if the Eth1Monitor is synced | ||
| ## to the finalization point. |
There was a problem hiding this comment.
Wasn't there a convention that function comments should use the imperative form (Return true if the Eth1Monitor is synced to the finalization point) instead of descriptive form (the current version) or the most common third person form (ReturnS true if ...)? I can't find it in the style-guide anymore but I remember seeing something suggesting imperative form after running into this style in a couple modules. Maybe it's also an Ethereum style rule (they use imperative as well).
Also, Indicate whether vs Return true if, to better describe the false case
There was a problem hiding this comment.
I've changed the wording because yesterday while reading the code I was confused for a moment that the comment was referring to the if statement below which didn't make sense.
#3944
The use of nested
awaitWithRetriescalls would haveresulted in an unexpected number of retries (3x3).
We now use regular
awaitin outer layer to avoid the problem.#3943
The new code has an invariant that the
headMerkleizerfield inthe
Eth1Chainis always kept in sync with the blocks stored inthe chain.
This invariant is now enforced better by doing the necessary merkleizer updates
in the
Eth1Chain.addBlockfunction, in theEth1Chain.initfunction and in theEth1Chain.resetfunction.