Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Gate advance-nonce change#11032

Closed
CriesofCarrots wants to merge 4 commits intosolana-labs:masterfrom
CriesofCarrots:gate-nonce-change
Closed

Gate advance-nonce change#11032
CriesofCarrots wants to merge 4 commits intosolana-labs:masterfrom
CriesofCarrots:gate-nonce-change

Conversation

@CriesofCarrots
Copy link
Copy Markdown
Contributor

Problem

As of #10973 , a successful nonce transaction advances the nonce to the most recent blockhash from the RecentBlockhashes sysvar instead of re-overwriting that nonce with the most recent bank blockhash. We expect those hashes to be the same, but in fact that can be different in the final tick of the slot, due to this:

if self.is_block_boundary(current_tick_height + 1) {

This caused a bank hash mismatch on our api nodes.

Summary of Changes

  • Gate the change by operating mode/slot

Fixes #11020

@CriesofCarrots CriesofCarrots requested a review from mvines July 13, 2020 18:40
@CriesofCarrots
Copy link
Copy Markdown
Contributor Author

@mvines , we might be able to avoid the churn of this gating by fixing the discrepancy directly: #11020 (comment)
Thoughts?

@mvines
Copy link
Copy Markdown
Contributor

mvines commented Jul 13, 2020

@mvines , we might be able to avoid the churn of this gating by fixing the discrepancy directly: #11020 (comment)
Thoughts?

Sounds promising. That would just move the gating into bank.rs but would be an improvement to the accounts plumbing fo sure

fn get_fix_nonce_overwrite_slot(operating_mode: OperatingMode) -> Slot {
match operating_mode {
OperatingMode::Development => std::u64::MAX / 2,
OperatingMode::Development => 0,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Made this change in order to unignore test_nonce_fee_calculator_updates

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 13, 2020

Codecov Report

Merging #11032 into master will increase coverage by 0.0%.
The diff coverage is 87.0%.

@@           Coverage Diff           @@
##           master   #11032   +/-   ##
=======================================
  Coverage    82.1%    82.2%           
=======================================
  Files         318      318           
  Lines       73249    73277   +28     
=======================================
+ Hits        60196    60236   +40     
+ Misses      13053    13041   -12     

@mergify mergify Bot closed this in #11036 Jul 14, 2020
@CriesofCarrots CriesofCarrots deleted the gate-nonce-change branch July 24, 2020 22:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nonce fee_calculator overwrite fix causes mismatched bank hash

2 participants