Skip to content

Ensure successful block submission even when sequencer crashes#442

Merged
jbowen93 merged 22 commits intomainfrom
jbowen93/write-consistency
Jun 24, 2022
Merged

Ensure successful block submission even when sequencer crashes#442
jbowen93 merged 22 commits intomainfrom
jbowen93/write-consistency

Conversation

@jbowen93
Copy link
Copy Markdown
Contributor

@jbowen93 jbowen93 commented Jun 16, 2022

Significant changes in this PR

  1. Modified the ApplyBlock method in state/executor.go splitting it into 2 methods.
    1. ApplyBlock which now only validates and executes the block/state
    2. Commit which commits the block
  2. Modifies when blocks are stored in publishBlock in block/manager.go
    1. Create a top level if/else statement that checks if there is a pendingBlock in the manager.store at the next block height. This ensures that if the sequencer crashes during publishBlock (generally during a slow submitBlockToDA call) it will use the already generated pendingBlock as opposed to creating a new block.
    2. Uses the new ApplyBlock/Commit methods to ApplyBlock prior to submitBlockToDA but only Commits the block after successful DA submission. Because ApplyBlock doesn't write to disk a crash is gracefully handled by reapplying the stored pendingBlock with the changes in (2.i)
  3. No longer panics when submitBlockToDA fails after reaching the maximum number of retries. Now returns an error.
  4. Updates github.com/dgraph-io/ristretto (used in Badger DB) to include this change which allows for an Ethermint node to be restarted after crashing on arm64
  5. Modifies SaveBlock in store/store.go so that it doesn't increment the store's height. Adds SetHeight to store/store.go and calls it immediately after submitBlockToDA within publishBlock in block/manager.go

Resolves: #437
Resolves: #447
Resolves: #450

P.S. My commit messages are obviously terrible but this PR will be squashed and we'll use the detailed PR description.

@jbowen93 jbowen93 self-assigned this Jun 16, 2022
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 16, 2022

Codecov Report

Merging #442 (cc0afcd) into main (66a5590) will increase coverage by 0.06%.
The diff coverage is 61.11%.

@@            Coverage Diff             @@
##             main     #442      +/-   ##
==========================================
+ Coverage   55.19%   55.25%   +0.06%     
==========================================
  Files          48       48              
  Lines        8751     8788      +37     
==========================================
+ Hits         4830     4856      +26     
- Misses       3177     3184       +7     
- Partials      744      748       +4     
Impacted Files Coverage Δ
rpc/client/client.go 46.46% <0.00%> (ø)
state/executor.go 70.79% <30.76%> (-0.50%) ⬇️
block/manager.go 62.46% <66.03%> (+0.08%) ⬆️
store/store.go 66.23% <100.00%> (+0.44%) ⬆️
rpc/json/service.go 65.64% <0.00%> (+1.84%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66a5590...cc0afcd. Read the comment docs.

@jbowen93 jbowen93 force-pushed the jbowen93/write-consistency branch from 58ae3ea to 14fa63d Compare June 21, 2022 13:09
Comment thread store/store.go Outdated
@jbowen93
Copy link
Copy Markdown
Contributor Author

Proposal to punt on fixing Ethermint rpc tests until #450

@jbowen93 jbowen93 marked this pull request as ready for review June 22, 2022 21:14
Copy link
Copy Markdown
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

Looks good. Left one question and an optional suggestion.

Comment thread block/manager.go
Comment thread block/manager.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants