Updated blockchainBridge hasStateForBlock to use config instead of st…

[mrzeszutko]

…ate. Relates to #9408

Changes

• Updated BlockChainBridge. HasStateForBlock to check the configuration and not call the state at all. The result is calculated based on the node type: archive node vs. full node, the information if we finish the initial sync and pruning boundary.
• To cater for the race condition, we return false at the pruning boundary (So we return that we don't have state when we are at the pruning boundary) - this is a safety margin

Types of changes

What types of changes does your code introduce?

• [ ] Bugfix (a non-breaking change that fixes an issue)
• [ ] New feature (a non-breaking change that adds functionality)
• [x] Breaking change (a change that causes existing functionality not to work as expected)
• [x] Optimization
• [ ] Refactoring
• [ ] Documentation update
• [ ] Build-related changes
• [ ] Other: Description

Testing

Requires testing

• [x] Yes
• [ ] No

If yes, did you write tests?

• [x] Yes
• [ ] No

Documentation

Remarks

There were no changes done to the snap serving. 128 is hardcoded there so there should be no impact

[asdacap]

I think the fundamental issue is that 'HasStateForBlockcheck inIWorldStateandIStateReader` is not correct. I was kinda not correct as the existance of the root node does not necessarily mean that the whole trie is available. Its also weird that the root node is not pruned randomly.

[asdacap]

Also, checkout LastNStateRootTracker.

[LukaszRozmej]

I think the fundamental issue is that 'HasStateForBlockcheck inIWorldStateandIStateReader` is not correct. I was kinda not correct as the existance of the root node does not necessarily mean that the whole trie is available. Its also weird that the root node is not pruned randomly.

Why the existance of root node doesn't imply the existence of the trie? That was basics of Trie in the past that we only persisted parent node if all it's children were persisted.

[asdacap]

Its children might get removed during in memory pruning. Thats why. Some of the nodes may not get removed due to memory limitation for tracking.

[LukaszRozmej]

Its children might get removed during in memory pruning. Thats why. Some of the nodes may not get removed due to memory limitation for tracking.

Sure but that is deviation from previous assumptions that were commonly used (like in RPC) and to be honest poses a potential for hard to track bugs (though might be necessary for better pruning)

[mrzeszutko]

I think the fundamental issue is that 'HasStateForBlockcheck inIWorldStateandIStateReader` is not correct. I was kinda not correct as the existance of the root node does not necessarily mean that the whole trie is available. Its also weird that the root node is not pruned randomly.

@asdacap is this relevant for this PR? For RPC calls we don't want to reference state, just base the response on config (e.g. pruning boundary etc)

[asdacap]

just base the response on config (e.g. pruning boundary etc)

The RPC consult the world state if a state is available or not. The world state is faulty. IMO this check is important especially for triestore as during memory pruning (around 1 to 2 second on mainnet probably every 30 minute), nodes are concurrently removed, so it definitely need to check config like what you did. However, think carefully, now it uses pruning boundary, later its gonna respect finalized block, later again its gonna have special archive node that only have last 1 mil block or something, do you really want to have all that logic in BlockchainBridge? Consider putting it in IWorldStateManager or IStateReader instead. Except for the sync check part. I guess that make sense here.

[asdacap]

I think you can simply check for IBlockTree.BestPersistedState, which is kinda the same as TrieStore.LastPersistedBlockNumber. It should already have pruning boundary considered. Except for archive mode, then just dont check.

[LukaszRozmej]

just base the response on config (e.g. pruning boundary etc)

The RPC consult the world state if a state is available or not. The world state is faulty. IMO this check is important especially for triestore as during memory pruning (around 1 to 2 second on mainnet probably every 30 minute), nodes are concurrently removed, so it definitely need to check config like what you did. However, think carefully, now it uses pruning boundary, later its gonna respect finalized block, later again its gonna have special archive node that only have last 1 mil block or something, do you really want to have all that logic in BlockchainBridge? Consider putting it in IWorldStateManager or IStateReader instead. Except for the sync check part. I guess that make sense here.

I would consider putting it in IPruningStrategy