near / nearcore

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

fix: Set DBCol::ReceiptIdToShardId in state sync and catchup #11668

Closed tayfunelmas closed 4 days ago

tayfunelmas commented 1 week ago

This addresses the issue in #11605. This is temporary change, we will follow-up with removing ReceiptIdToShardId as it is not needed except for testing. It allows to re-enable GC in testloop.

For this we fix 2 problems (both manifest itself as testloop failures): 1) Call save_receipt_id_to_shard_id_for_block in 2 new paths in state sync (set_state_finalize) and catchup (block_catch_up_postprocess). This makes sure that ReceiptIdToShardId is saved whenever OutgoingReceipts are saved. 2) Only decrement refcount of ReceiptIdToShardId if it exists in GC. This is needed because when saving ReceiptIdToShardId we check if the validator cares about the shard, but when garbage collecting, we do not check shard-tracking info.

We will follow-up with another change to remove ReceiptIdToShardId from DB at all, since it is currently not used except for testing.

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 80.85106% with 9 lines in your changes missing coverage. Please review.

Project coverage is 71.67%. Comparing base (642f6a0) to head (ec12848).

Files Patch % Lines
chain/chain/src/chain.rs 70.00% 0 Missing and 3 partials :warning:
chain/chain/src/garbage_collection.rs 77.77% 2 Missing :warning:
chain/chain/src/chain_update.rs 90.00% 0 Missing and 1 partial :warning:
chain/client/src/sync/state.rs 75.00% 0 Missing and 1 partial :warning:
integration-tests/src/test_loop/builder.rs 90.90% 1 Missing :warning:
tools/state-viewer/src/state_parts.rs 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11668 +/- ## ========================================== - Coverage 71.67% 71.67% -0.01% ========================================== Files 788 788 Lines 161374 161402 +28 Branches 161374 161402 +28 ========================================== + Hits 115669 115677 +8 - Misses 40672 40686 +14 - Partials 5033 5039 +6 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11668/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/11668/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/11668/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/11668/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.35% <0.00%> (-0.01%)` | :arrow_down: | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11668/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `37.87% <78.72%> (+0.01%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11668/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `69.06% <80.85%> (-0.01%)` | :arrow_down: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11668/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.16% <80.85%> (-0.01%)` | :arrow_down: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11668/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `52.63% <13.88%> (+1.55%)` | :arrow_up: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11668/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/11668/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.38% <0.00%> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11668/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.25% <13.88%> (-0.02%)` | :arrow_down: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11668/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 1 week ago

We will follow-up with another change to remove ReceiptIdToShardId from DB at all, since it is currently not used except for testing.

Why not do that right away?

tayfunelmas commented 1 week ago

We will follow-up with another change to remove ReceiptIdToShardId from DB at all, since it is currently not used except for testing.

Why not do that right away?

I just had this PR ready when I discovered we could just delete the column. I will prepare the other PR and if it ends up quick, will scratch this one.

tayfunelmas commented 4 days ago

Closing this PR on behalf of https://github.com/near/nearcore/pull/11691.