Skip to content

node: fix genesis on start up#6563

Merged
cmwaters merged 5 commits intomasterfrom
callum/fix-startup
Jun 10, 2021
Merged

node: fix genesis on start up#6563
cmwaters merged 5 commits intomasterfrom
callum/fix-startup

Conversation

@cmwaters
Copy link
Contributor

@cmwaters cmwaters commented Jun 9, 2021

After observing a few failed e2e tests I realised that I missed an edge case (when genesis has no validators) for this PR: #6554.

Copy link
Contributor

@tychoish tychoish left a comment

Choose a reason for hiding this comment

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

We currently have a genesis state store within the node, which seems quirky without having more context. Is there a path to being able to avoid this? https://github.com/tendermint/tendermint/blob/master/node/node.go#L75

How much of the e2e failures does this account for? Should it be backported to 0.34?

node/setup.go Outdated
evidencePool, err := evidence.NewPool(logger, evidenceDB, sm.NewStore(stateDB), blockStore)
if err != nil {
return nil, nil, nil, err
return nil, nil, nil, fmt.Errorf("creating evidence pool: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be a %w

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct

@cmwaters
Copy link
Contributor Author

cmwaters commented Jun 9, 2021

We currently have a genesis state store within the node, which seems quirky without having more context. Is there a path to being able to avoid this?

We probably don't need this since we can derive it from the genesis doc

@codecov
Copy link

codecov bot commented Jun 10, 2021

Codecov Report

Merging #6563 (63204dd) into master (4e59575) will increase coverage by 0.11%.
The diff coverage is 8.33%.

@@            Coverage Diff             @@
##           master    #6563      +/-   ##
==========================================
+ Coverage   61.03%   61.14%   +0.11%     
==========================================
  Files         295      295              
  Lines       27868    27850      -18     
==========================================
+ Hits        17008    17029      +21     
+ Misses       9139     9108      -31     
+ Partials     1721     1713       -8     
Impacted Files Coverage Δ
node/node.go 51.69% <0.00%> (-0.17%) ⬇️
node/setup.go 66.74% <0.00%> (ø)
rpc/jsonrpc/server/http_json_handler.go 61.48% <0.00%> (ø)
state/store.go 56.80% <ø> (+3.71%) ⬆️
state/state.go 84.47% <100.00%> (ø)
internal/statesync/block_queue.go 89.71% <0.00%> (-4.68%) ⬇️
types/tx.go 82.97% <0.00%> (-4.26%) ⬇️
internal/statesync/dispatcher.go 78.35% <0.00%> (-1.50%) ⬇️
internal/p2p/transport_mconn.go 81.28% <0.00%> (-0.99%) ⬇️
internal/p2p/switch.go 60.18% <0.00%> (-0.47%) ⬇️
... and 7 more

@cmwaters cmwaters merged commit 32bc399 into master Jun 10, 2021
@cmwaters cmwaters deleted the callum/fix-startup branch June 10, 2021 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants