celestiaorg / celestiaorg/celestia-node

feat(share/p2p/peer-manager): Rework pool validation

Open
#3,011 1 comment 0 reactions 0 assignees View on GitHub
area:p2p kind:refactor shrex stale
Dominant language
Go
Stars
996
Forks
1.1k
Avg merge
1d 6h
Merged PRs (30d)
34

Description

### Implementation ideas

This issue contains idea to rework peer-manager pool validation from timeout based to comparing directly with headers using internal height index.

Recently peer-manager became aware of header height of peer pools in all of its interfaces. It allows to simplify logic of validation of peer pools. Current approach is based on creating a `createdAt` timestamp of recieving datahash for first time. If datahash happen to be also received from headerSub within timeout, the datahash is marked as valid. If datahash is not validated in timeout, it will get blacklisted with all peers that have sent messages with this datahash.

Now this logic can be greatly simplified. It requires few changes that will allow great code cleanup.:
### 1. Store window
Peer manager should only store datahashes for height within store window. Store window can be set with some delta around last headerSub header height.
```
storeTo := lastHeaderHeight + const
storeFrom := lastHeaderHeight - const

if msg.Height < storeFrom || msg.Height > storeTo{
ignore
}
```

2. Height index
Peer-manager needs new fields for storing height -> datahash index of maintained peer pools.
```
heightToVerification map[uint]verification

type verification struct {
validHash string
submittedHashes map[string]struct
```
`validHash` is a source of truth datahash. It should be filled headerSub msg once it is available. Before it is available all hashes has to be collected into `submittedHashes` map. Once `validHash` is received, all other hashes has to be blacklisted immediately. After `validHash` is filled all messages can be validated in place by comparing msg datahash with `validHash`.

With those changes time based verification can be removed, aswell as `Validated` field and all related async validation logic can be removed from `pool` struct

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.