Applied PR comments

This commit is contained in:
Jasmina Malicevic
2022-06-22 08:14:30 +02:00
parent 92235fc2b6
commit e266b97396
5 changed files with 27 additions and 28 deletions
+1 -1
View File
@@ -33,7 +33,7 @@ function testExample() {
exit 1
fi
rm "${INPUT}".out.new
# rm "${INPUT}".out.new
}
testExample 1 tests/test_cli/ex1.abci abci-cli kvstore
+6 -6
View File
@@ -12,11 +12,11 @@ On startup, the reactor fires up four go routines:
3. Handle p2p channel messages
4. Process peer updates
The pool routine picks out blocks form the block pool and processes them. It also checks whether the node should switch to consensus.
The pool routine picks out blocks form the block pool and processes them. It also checks whether the node should switch to consensus.
All messages that go through the p2p channel are processed within the `processBlockSyncCh` routine.
On peer update messages the reactor adds or removes the sending peer.
On peer update messages the reactor adds or removes the peer that has sent the update message.
**Note** There is currently a check for whether we have a message from an empty peer.
@@ -33,7 +33,7 @@ On peer update messages the reactor adds or removes the sending peer.
- Channel id: 0x40
- Size of receive buffer: 1024 messages
- Size of send queue: 1000 messages
- Message size: maximum size of a block + size of proto block messages (response message prefix and key size)
- Message size: maximum size of a block + size of proto block messages (response message prefix and key size) + size of the Extended commit.
Messages processed via the channel:
| Message name | Message fields| Description |
@@ -42,7 +42,7 @@ Messages processed via the channel:
| `BlockResponse`| `block types.Block `| Send `block` to peer that requested it |
| `NoBlockResponse` |`height int64`|Indicates that a peer does not have a block at `height`|
|`StatusRequest`| `{} `| Sent to a peer to request its status|
|`StatusResponse` | `height int64, base int64`|Send to a peer the lowest and heights height of blocks within it's store (`store.Height()`, `store.Base()`)|
|`StatusResponse` | `height int64, base int64`|Send to a peer the lowest and heights height of blocks within its store (`store.Height()`, `store.Base()`)|
|`HeaderRequest` | `height: int64`| Request a header from peer for verification|
|`HeaderResponse` |`header: Header`| Return the header for the corresponding height|
@@ -75,7 +75,7 @@ When a block is received by a requester, the requester does a number of checks o
In the code there is the following ToDo listed:
` // TODO: ensure that blocks come in order for each peer.` This needs further specification.
If the checks pass, the `block` field of the requester is populated with the new block and i sthus made available to the blocksync reactor.
If the checks pass, the `block` field of the requester is populated with the new block and is thus made available to the blocksync reactor.
#### `Reactor`
@@ -85,6 +85,6 @@ If the checks pass, the `block` field of the requester is populated with the new
|`errorsCh`| `peerError`| size `maxPeerErrBuffer`|
|`didProcessCh`|`struct{}`| size `1`|
The channel is created within the pool routine of the reactor and is used to signal that the reactor should check the block pool for new blocks. A message is sent to the channel after a fixed timeout (`trySyncTicker`). As we need two blocks to verify one of them (this is more clearly defined in [verification](./verification.md), if we miss only on of them, we will not wait for the sync timer to time out, but rather try quickly again until we fetch both.
The channel is created within the pool routine of the reactor and is used to signal that the reactor should check the block pool for new blocks. A message is sent to the channel after a fixed timeout (`trySyncTicker`). As we need two blocks to verify one of them (this is more clearly defined in [verification](./verification.md), if we miss only one of them, we will not wait for the sync timer to time out, but rather try quickly again until we fetch both.
`switchToConsensusTicker`. In addition to the sync timeout, in the same routine, the reactor checks periodically whether the conditions to switch to consensus are fullfilled.
+1 -1
View File
@@ -44,7 +44,7 @@ type TrustedBlockData struct {
commit *types.Commit
}
```
The block pool stores the last executed block(`height`), keeps track of peers connected to a node, the current height for each peer, along with the number of pending requestes for each peer and assigns requests to peers (by creating `requesters`).
The block pool stores the last executed block (`height`), keeps track of peers connected to a node, the current height for each peer, along with the number of pending requests for each peer and assigns requests to peers (by creating `requesters`).
```go
type BlockPool {
+10 -7
View File
@@ -14,10 +14,10 @@ many blocks (that have already been decided) in parallel, verifying their commit
ABCI application.
Tendermint full nodes run the Blocksync Reactor as a service to provide blocks
to new nodes. New nodes run the Blocksync Reactor in "fast_sync" mode,
to new or recovering nodes. The nodes run the Blocksync Reactor in "fast_sync" mode,
where they actively make requests for more blocks until they sync up.
Once caught up, "fast_sync" mode is disabled and the node switches to
using the Consensus Reactor.
using the Consensus Reactor.
*Note* It is currently assumed that the Consensus reactor is already running. It is therefore not turned on by the Blocksync reactor. In case it has not been started, the Blocksync reactor simply returns.
@@ -27,18 +27,20 @@ A node can switch to blocksync directly on start-up or after completing `state-s
The blocksync reactor service is started at the same time as all the other services in Tendermint. But blocksync-inc is disabled (blockSync boolean flag is false) initially and thus the blockpool and the routine to process blocks from the pool are not launched until the reactor is actually activated.
The reactor is actived after state sync, where the pool and request processing routines are launched.
The reactor is activated after state sync, where the pool and request processing routines are launched.
However, receiving messages via the p2p channel and sending status updates to other nodes is enabled regardless of whether the blocksync reactor is started. This makes sense as a node should be able to send updates to other peers regardless of whether it itself is blocksyncing.
**Note**. In the current version, if we start from state sync and block sync is not launched before as a service, the internal channels used by the reactor will not be created. We need to be careful to launch the blocksync *service* before we call the function to switch from statesync to blocksync.
### Switching from blocksync to consensus
Ideally, the switch to consensus is done either after we have caught up to the maximum height reported by a peer or we have not advanced our height for more than 60s.
Ideally, the switch to consensus is done once the node considers itself caugh up or we have not advanced our height for more than 60s.
The former is checked by calling `isCaughtUp` inside `poolRoutine` periodically. This period is set with `switchToConsensusTicker`. We consider a node to be caught up if it is 1 height away from the maximum height reported by its peers. The reason we **do not catch up until the maximum height** (`pool.maxPeerHeight`)is that we cannot verify block at `pool.maxPeerHeight` without the `lastCommit` of the block at `pool.maxPeerHeight + 1`.
The former is checked by calling `isCaughtUp` inside `poolRoutine` periodically. This period is set with `switchToConsensusTicker`. We consider a node to be caught up if it is 1 height away from the maximum height reported by its peers. The reason we **do not catch up until the maximum height** (`pool.maxPeerHeight`) is that we cannot verify the block at `pool.maxPeerHeight` without the `lastCommit` of the block at `pool.maxPeerHeight + 1`.
Blocksync **does not** switch to consensus until we have synced at least one block. We need to have vote extensions in order to participate in consensus and they are not provided to the blocksync reactor after state sync. We therefore need to receive them from one of our peers.
If the node is not starting from genesis, blocksync **does not** switch to consensus until we have synced at least one block. We need to have vote extensions in order to participate in consensus and they are not provided to the blocksync reactor after state sync. We therefore need to receive them from one of our peers.
When the node is starting from genesis, the first block does not need the vote extensions and is able to switch directly to consensus.
## Architecture and algorithm
@@ -46,7 +48,8 @@ The Blocksync reactor is organised as a set of concurrent tasks:
- Receive routine of Blocksync Reactor
- Task for creating Requesters
- Set of Requester tasks and - Controller task.
- Set of Requester tasks
- A controller task.
![Blocksync Reactor Architecture Diagram](img/bc-reactor.png)
+9 -13
View File
@@ -3,7 +3,7 @@
When blocksyncing, a node is not participating in conensus. It is receiving blocks that have already been decided and committed. To avoid being fooled by malicious peers, the node has to verify the received blocks before executing the transactions and storing the block in its store.
The verification in blocksync aims to apply the same logic as the [light client verification](../light-client/verification/README.md). The safety guarantees of the light client model are provided by verifiying blocks starting from an **initial trusted state** and using validators who were bonded within a **trusting period**. Once this period expires, the validators could have unbonded, and we cannot rely on them being hoenst anymore. In blocksync, we can be **outside** the trusting period. Therefore, the guarantees provided by blocksync verification can be divided in two groups:
The verification in blocksync aims to apply the same logic as the [light client verification](../light-client/verification/README.md). The safety guarantees of the light client model are provided by verifying blocks starting from an **initial trusted state** and using validators who were bonded within a **trusting period**. Once this period expires, the validators could have unbonded, and we cannot rely on them being honest anymore. In blocksync, we can be **outside** the trusting period. Therefore, the guarantees provided by blocksync verification can be divided in two groups:
- The block is within the trusting period, we can punish validators, and provide guarantees that if a block is signed by trusted validators, they are indeed correct.
- The block is outside the trusting period and we cannot guarantee that the validators are still honest. We can however make a potential attack by those validators more complicated by additionally verifying the block using other peers as *witnesses*.
@@ -14,14 +14,12 @@ A state becomes trusted once it passes the validation. The section below discuss
Currently there are two possible ways to obtain a trusted state :
1. If the node is blocksync-ing from genesis, we have to verify the very first block and mark it as the initial trusted state.
[//]: # (trust the validators provided in the initial state and use them to verify the initial block received from peers.)
2. The node loads the last block stored in its local block store.
**Case 1 - blocksync from genesis**
In this case we only verify that the validator hash of the block matches the validator set of the initial state. We will use the initially provided validator set for verification and further verify the block against witne+sses. In this scenario, this state is very likely to be outside the trusting period. We will accept this block as a trusted state and store it inside the node's block store.
In this case we only verify that the validator hash of the block matches the validator set resulting from executing `InitChain`. We will use the initially provided validator set for verification and further verify the block against witnesses. In this scenario, this state is very likely to be outside the trusting period. We will accept this block as a trusted state and store it inside the node's block store.
It is worth noting that, running block sync from the first height is significantly slower than running statesync first. However, statesync does not keep the entire blockchain history and some operators might opt not to state sync. The reason is that, if sufficiently many nodes state sync and other nodes who have historical data fail or leave the network, we have no history.
@@ -55,26 +53,24 @@ The diagram above shows all blocks at play when verifying block at height ` H +
At a high level, the verification algorithm is represented by the code below.
~~~golang
if LastCommit.verify(trustedBlock) {
if !LastCommit.verify(trustedBlock) && NewBlock.validatorSet == trustedState.validatorSet {
// NewBlock is invalid, request new
request_newBlock_from_other_peer(height: h + 1)
} else {
if NewBlock.validatorSet == trustedState.validatorSet {
if LastCommit2.validatorSetHash == trustedState.validatorSetHash &&
} else {
if LastCommit2.validatorSetHash == trustedState.validatorSetHash &&
verifyBlock.validatorSetHash == trustedState.nextValidatorSetHash {
if LastCommit2.verify(NewBlock) {
if verify_block_with_peers(NewBlock) {
ApplyBlock(NewBlock)
}
}|
}
} else
find_new_verifyBlock(height: h + 2)
}
}
~~~
We try to increase the trust in the NewBlock by first confirming that it's `lastCommit` field indeed verifies the trusted block. We then confirm that the validator hash matches the expected hash. But to fully verify the block we need to check it against the signatures that signed it. They are stored within the `lastCommit2` of `verifyBlock` at height `H+2`. As we have no gurantees on this block, we again increase our trust in it by confirming that the validators whose signatures are in `lastCommit2` are a match for the expected validator set.
We try to increase the trust in the NewBlock by first confirming that its `lastCommit` field indeed verifies the trusted block. We then confirm that the validator hash matches the expected hash. But to fully verify the block we need to check it against the signatures that signed it. They are stored within the `lastCommit2` of `verifyBlock` at height `H+2`. As we have no guarantees on this block, we again increase our trust in it by confirming that the validators whose signatures are in `lastCommit2` are a match for the expected validator set.
#### *Witness verification*
@@ -110,8 +106,8 @@ We have no guarantees on the correctness of the peers we are connected to.
- Once we replace a peer due to it having timed out or not reporting a correct block, nothing prevents a node from reconnecting to it - *there is no notion of blacklisted peers*.
- If we connect to a subset of peers, they could feed the node faulty data. Eventually, when the node switches to consensus, it would realize there is something wrong, but then the node itself might be blacklisted.
- Alternatively, a node that is fed faulty data, could, upon switching to conensus, become part of a light client attack by serving as a faulty witness.
- There is no check whether the maximum height reported by peers is true or not. A slow node could report a very distant height to the node - for example 2000, when the blockchain is at height 1000 in fact. This would lead to one part of the condition to switch to consensus never being true. To prevent the node switching due to not advancing, the malicious node sends a new block very slowly. Thus the node progresses but can never participate in consensus. This issue could potentially be mitigated if, instead of taking the maximum height reported by peers, we report the lowest of their maximums. The idea is that peers should be close enought to the top of the chain in any case.
- A blocksyncing node can flood peers with requests - constantly reporting that it has not synced up. At the moment the maximum amount of requests received is limited and protects peers to some extend against this attack.
- There is no check whether the maximum height reported by peers is true or not. A slow node could report a very distant height to the node - for example 2000, when the blockchain is at height 1000 in fact. This would lead to one part of the condition to switch to consensus never being true. To prevent the node switching due to not advancing, the malicious node sends a new block very slowly. Thus the node progresses but can never participate in consensus. This issue could potentially be mitigated if, instead of taking the maximum height reported by peers, we report the lowest of their maximums. The idea is that peers should be close enough to the top of the chain in any case.
- A blocksyncing node can flood peers with requests - constantly reporting that it has not synced up. At the moment, the maximum amount of requests received is limited and protects peers to some extent against this attack.
## Additional ideas and suggestions