Rework lowest block height in storage #2789

Fraser999 · 2022-03-25T03:17:37Z

This PR replaces the disjoint sequence tracking in storage with a persisted lowest_contiguous_block_height variable. The value represents the lowest block from which the node has contiguous blocks (and corresponding global state) to the current chain tip.

It is updated after node startup when the chain synchronization has retrieved the trusted block. In sync-to-genesis mode, the value is set to 0 and in fast-sync mode, it is set to the height of ~~the trusted block~~ the most recent block.

The value is only actually changed and written to the DB if:

it doesn't previously exist, or
the new height is lower than the current one, or
the new height is higher than the highest block stored at node startup (before chain sync commenced)

A new AvailableBlockRange struct was added to avoid using (u64, u64) to represent the range. It is more explicit in the code and in the JSON-RPC error response:

"error": {
  "code": -32001,
  "message": "block at height 1314 not stored on this node",
  "data": {
    "available_block_range": {
      "low": 75,
      "high": 112
    }
  }
}

Closes #2779.

node/src/components/chain_synchronizer/operations.rs

rafal-ch · 2022-03-25T12:16:09Z

node/src/components/rpc_server/rpcs/common.rs

-    pub(super) fn highest_contiguous_block_height_range(low: u64, high: u64) -> Self {
-        ErrorData::HighestContiguousBlockHeightRange((low, high))
-    }
+    HighestContiguousBlockHeightRange(ContiguousBlockRange),


Since we now have a single range, it no longer needs to be called Highest (also in the comment).

As discussed, I'll rename to AvailableBlockRange.

Updated in b834b44.

node/src/components/chain_synchronizer/operations.rs

rafal-ch

LGTM 👍

node/CHANGELOG.md

goral09

Minor comments but no blockers.

node/src/components/chain_synchronizer/operations.rs

goral09 · 2022-03-25T15:45:10Z

node/CHANGELOG.md

@@ -36,7 +36,7 @@ All notable changes to this project will be documented in this file.  The format
 * Connection handshake timeouts can now be configured via the `handshake_timeout` variable (they were hardcoded at 20 seconds before).
 * `Key::SystemContractRegistry` is now readable and can be queried via the RPC.
 * Requests for data from a peer are now de-prioritized over networking messages necessary for consensus and chain advancement.
-* JSON-RPC responses which fail to provide requested data will now also include an indication of that node's highest contiguous block height range, i.e. the block heights for which it holds all global state
+* JSON-RPC responses which fail to provide requested data will now also include an indication of that node's available block range, i.e. the block heights for which it holds all global state.  For nodes running with `[node.sync_to_genesis]` set to true, the range will be the full blockchain, otherwise the range will start at a block near the tip of the chain when the node started running.


Suggested change

* JSON-RPC responses which fail to provide requested data will now also include an indication of that node's available block range, i.e. the block heights for which it holds all global state. For nodes running with `[node.sync_to_genesis]` set to true, the range will be the full blockchain, otherwise the range will start at a block near the tip of the chain when the node started running.

* JSON-RPC responses which fail to provide requested data will now also include an indication of that node's available block range, i.e. the block heights for which it holds all global state. For nodes running with `[node.sync_to_genesis]` set to true, the range will be the full blockchain, otherwise the range will start at a block near the tip of the chain when the node started running.

😎

I have conflicts to sort out in the changelog which I plan to do once #2788 also merges. I'll fix this when I do that :)

goral09 · 2022-03-25T16:59:56Z

node/src/components/storage.rs

+        let low = self.lowest_available_block_height;
+        match AvailableBlockRange::new(low, high) {
+            Ok(range) => range,
+            Err(error) => {


The only possible error here is when high is actually lower than low. Maybe it makes sense here to flip them? It seems to be harmless and maybe the problem would "work itself out" until the next query (if the index and/or lowest_available_block_height get updated).

I think I'd rather leave as is. It's really pointing to a bug in our core logic if that actually arises (but not something worth stopping the node over). So the default range [MAX, MAX] being returned to the client is probably a better indication that something is very wrong server side than asking for e.g. block 150 and getting back an error saying the server's available blocks are 100 -> 200.

goral09 · 2022-03-25T17:02:14Z

node/src/components/storage.rs

+        // We should update the value if
+        // * it wasn't already stored, or
+        // * the new value is lower than the available range at startup, or
+        // * the new value is 2 or more higher than (after pruning due to hard-reset) the available


nitpick: maybe it's worth including an explanation why is that a problem since it may not be obvious to the reader. I.e. just write that there would be a single-block gap in the sequence where we might not have the Global State.

…ont-block-height

Fraser999 · 2022-03-25T21:09:29Z

bors merge

casperlabs-bors-ng · 2022-03-25T21:51:16Z

Build succeeded:

continuous-integration/drone/push

rework lowest block height in storage

80fb014

Fraser999 requested review from xcthulhu, fizyk20, goral09, afck and rafal-ch March 25, 2022 03:17

rafal-ch mentioned this pull request Mar 25, 2022

Check block availability when responding to RPC requests #2788

Merged

rafal-ch reviewed Mar 25, 2022

View reviewed changes

rename highest contiguous block range to available block range

b834b44

rafal-ch approved these changes Mar 25, 2022

View reviewed changes

goral09 reviewed Mar 25, 2022

View reviewed changes

node/CHANGELOG.md Outdated Show resolved Hide resolved

Fraser999 added 2 commits March 25, 2022 15:16

correct the lowest available block height for fast-sync case

e8486b1

fix the update condition for the lowest available block height

9b0ba5e

goral09 approved these changes Mar 25, 2022

View reviewed changes

Fraser999 added 3 commits March 25, 2022 18:07

Merge remote-tracking branch 'upstream/dev' into 2779-change-lowest-c…

b9cb5c4

…ont-block-height

cleanup after merge

e1a8f6c

update storage tests to reflect updated behavior

5e2c375

casperlabs-bors-ng bot merged commit 4e6367f into casper-network:dev Mar 25, 2022

Fraser999 deleted the 2779-change-lowest-cont-block-height branch March 25, 2022 22:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework lowest block height in storage #2789

Rework lowest block height in storage #2789

Fraser999 commented Mar 25, 2022 •

edited

Loading

rafal-ch Mar 25, 2022

Fraser999 Mar 25, 2022

Fraser999 Mar 25, 2022

rafal-ch left a comment

goral09 left a comment

goral09 Mar 25, 2022

Fraser999 Mar 25, 2022 •

edited

Loading

goral09 Mar 25, 2022

Fraser999 Mar 25, 2022

goral09 Mar 25, 2022

Fraser999 commented Mar 25, 2022

casperlabs-bors-ng bot commented Mar 25, 2022

	* JSON-RPC responses which fail to provide requested data will now also include an indication of that node's available block range, i.e. the block heights for which it holds all global state. For nodes running with `[node.sync_to_genesis]` set to true, the range will be the full blockchain, otherwise the range will start at a block near the tip of the chain when the node started running.
	* JSON-RPC responses which fail to provide requested data will now also include an indication of that node's available block range, i.e. the block heights for which it holds all global state. For nodes running with `[node.sync_to_genesis]` set to true, the range will be the full blockchain, otherwise the range will start at a block near the tip of the chain when the node started running.

Rework lowest block height in storage #2789

Rework lowest block height in storage #2789

Conversation

Fraser999 commented Mar 25, 2022 • edited Loading

rafal-ch Mar 25, 2022

Choose a reason for hiding this comment

Fraser999 Mar 25, 2022

Choose a reason for hiding this comment

Fraser999 Mar 25, 2022

Choose a reason for hiding this comment

rafal-ch left a comment

Choose a reason for hiding this comment

goral09 left a comment

Choose a reason for hiding this comment

goral09 Mar 25, 2022

Choose a reason for hiding this comment

Fraser999 Mar 25, 2022 • edited Loading

Choose a reason for hiding this comment

goral09 Mar 25, 2022

Choose a reason for hiding this comment

Fraser999 Mar 25, 2022

Choose a reason for hiding this comment

goral09 Mar 25, 2022

Choose a reason for hiding this comment

Fraser999 commented Mar 25, 2022

casperlabs-bors-ng bot commented Mar 25, 2022

Fraser999 commented Mar 25, 2022 •

edited

Loading

Fraser999 Mar 25, 2022 •

edited

Loading