near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.31k stars 619 forks source link

fix(congestion_control) - Correctly initialize the congestion info in genesis #11642

Closed wacban closed 3 months ago

wacban commented 3 months ago

Correctly initialize the congestion info in the genesis chunks. The congestion info needs to be computed based on the state root of each chunk. The boostrapping method is reused for that purpose.

Just to be clear, the congestion info is not stored in the genesis. It is recomputed when the node is started and the genesis is loaded.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 88.00000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 71.64%. Comparing base (57c9796) to head (7d77d69). Report is 8 commits behind head on master.

Files Patch % Lines
chain/chain/src/chain.rs 80.35% 2 Missing and 9 partials :warning:
chain/chain/src/test_utils/kv_runtime.rs 0.00% 6 Missing :warning:
genesis-tools/genesis-populate/src/lib.rs 88.88% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11642 +/- ## ========================================== + Coverage 71.57% 71.64% +0.07% ========================================== Files 787 787 Lines 160728 161048 +320 Branches 160728 161048 +320 ========================================== + Hits 115039 115384 +345 + Misses 40648 40617 -31 - Partials 5041 5047 +6 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | Coverage Δ | | |---|---|---| | [backward-compatibility](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [db-migration](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [genesis-check](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.36% <0.00%> (-0.01%)` | :arrow_down: | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `37.81% <72.00%> (+0.08%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `69.03% <82.00%> (+0.03%)` | :arrow_up: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.14% <88.00%> (+0.07%)` | :arrow_up: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `52.61% <75.35%> (+1.64%)` | :arrow_up: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.59% <0.00%> (-0.01%)` | :arrow_down: | | [sanity-checks](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.39% <0.00%> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.23% <88.00%> (+0.09%)` | :arrow_up: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11642/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.28% <0.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wacban commented 3 months ago

I tested it manually in the following way:

wacban commented 3 months ago

I didn't add any new tests but the dumping logic is at least partially covered by some existing tests. I know because I broke them temporarily while implementing this :).

For completeness I also tested master using the same steps and got the error I was expecting:

thread '<unnamed>' panicked at chain/chain/src/runtime/mod.rs:412:21:
RuntimeError::UnexpectedIntegerOverflow remove_delayed_receipt_gas
stack backtrace:
   0: rust_begin_unwind
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/panicking.rs:72:14
   2: near_chain::runtime::NightshadeRuntime::process_state_update::{{closure}}
VanBarbascu commented 3 months ago

In you testing, can you consider that the state was dumped using a binary without these changes? Our usecase is that we take an old dump image and generate a genesis from it.