Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove block.Status #3158

Merged
merged 31 commits into from
Jul 16, 2024
Merged

Remove block.Status #3158

merged 31 commits into from
Jul 16, 2024

Conversation

StephenButtolph
Copy link
Contributor

@StephenButtolph StephenButtolph commented Jun 30, 2024 •
edited

Why this should be merged

This PR is the culmination of:

  1. Remove .Status() from .IsPreferred() #3111
  2. Remove Decided from the Consensus interface #3123
  3. Remove .Status() from .Accepted() #3124
  4. Refactor event.Blocker into job.Scheduler #3125
  5. Remove block lookup from deliver #3130
  6. Simplify dependency registration #3139
  7. Replace wasIssued with shouldIssueBlock #3131
  8. Remove parent lookup from issue #3132
  9. Remove status usage from consensus #3140

By removing Status from the snowman.Block interface, VMs no longer need to implement this method; which has historically been the source of many VM bugs.

How this works

By removing the last usage of snowman.Block#Status in the bootstrapping engine, the function can be removed from the interface and all VM implementations.

How this was tested

  • CI
  • Fuji Sync
    • Reduced the number of DB reads when bootstrapping the P-chain by 10% (from 1,403,721 to 1,268,418)
  • Fuji Probe
  • Mainnet Probe
  • Fuji Canary
  • Mainnet Canary

StephenButtolph
StephenButtolph commented Jul 1, 2024
View reviewed changes
snow/snowtest/decidable.go Show resolved Hide resolved
vms/example/xsvm/chain/block.go Outdated Show resolved Hide resolved
vms/proposervm/post_fork_option.go Show resolved Hide resolved
vms/proposervm/vm.go Show resolved Hide resolved
@StephenButtolph StephenButtolph added this to the v1.11.10 milestone Jul 3, 2024
@StephenButtolph StephenButtolph self-assigned this Jul 3, 2024
@StephenButtolph StephenButtolph added the consensus This involves consensus label Jul 4, 2024
@StephenButtolph StephenButtolph marked this pull request as ready for review July 6, 2024 01:13
marun
marun reviewed Jul 10, 2024
View reviewed changes
snow/consensus/snowman/snowmantest/engine.go Outdated Show resolved Hide resolved
snow/snowtest/status.go Show resolved Hide resolved
snow/engine/snowman/getter/getter.go Show resolved Hide resolved
vms/proposervm/vm_byzantine_test.go Outdated Show resolved Hide resolved
vms/components/chain/state_test.go Outdated Show resolved Hide resolved
vms/components/chain/state_test.go Outdated Show resolved Hide resolved
vms/example/xsvm/chain/block.go Show resolved Hide resolved
vms/proposervm/vm_test.go Show resolved Hide resolved
@ava-labs ava-labs deleted a comment from StephenButtolph Jul 16, 2024
tsachiherman
tsachiherman approved these changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@tsachiherman tsachiherman 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. just one comment above.

@StephenButtolph StephenButtolph added this pull request to the merge queue Jul 16, 2024
Merged via the queue into master with commit abb1a9a Jul 16, 2024
20 checks passed
@StephenButtolph StephenButtolph deleted the remove-block-status-for-real branch July 16, 2024 17:27
@aaronbuchwald aaronbuchwald mentioned this pull request Jul 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marun marun marun approved these changes

@dhrubabasu dhrubabasu dhrubabasu approved these changes

@joshua-kim joshua-kim joshua-kim approved these changes

@tsachiherman tsachiherman tsachiherman approved these changes

@abi87 abi87 Awaiting requested review from abi87

@danlaine danlaine Awaiting requested review from danlaine

Assignees

@StephenButtolph StephenButtolph

Labels
consensus This involves consensus
Projects
Platform Engineering Group
Archived in project
Milestone
v1.11.10
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@StephenButtolph @marun @dhrubabasu @joshua-kim @tsachiherman @aaronbuchwald