header: Extract Head method into separate Head interface, make Syncer implement it
#978
Conversation
renaynay
force-pushed
the
head-interface
branch
from
374f951 to
cdce25b
Compare
renaynay
force-pushed
the
head-interface
branch
from
cdce25b to
6156168
Compare
Codecov Report
@@ Coverage Diff @@
## main #978 +/- ##
==========================================
+ Coverage 58.56% 58.79% +0.23%
==========================================
Files 131 132 +1
Lines 7766 7985 +219
==========================================
+ Hits 4548 4695 +147
- Misses 2745 2814 +69
- Partials 473 476 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. |
node/services/config.go
Outdated
| @@ -22,6 +22,8 @@ type Config struct { | |||
| // Note: The trusted does *not* imply Headers are not verified, but trusted as reliable to fetch headers | |||
| // at any moment. | |||
| TrustedPeers []string | |||
| // BlockTime is the block time of the network. | |||
| BlockTime time.Duration | |||
There was a problem hiding this comment.
I am still kind of iffy on this being a config value. While it doesn't really impact anything other than the frequency at which the syncer requests a new objective head from its trustedPeer, if we do end up using this config param elsewhere in the code, we would have to be very cautious on how it is used. Wdyt @Wondertan ?
There was a problem hiding this comment.
We should not make it part of the config value but a part of params pkg(#790). Taken from the params, the value then gets pass to the Syncer
There was a problem hiding this comment.
Similarly, I don't think we should make it an option on the node, as by now.
| @@ -13,6 +14,8 @@ const ( | |||
| // Private can be used to set up any private network, including local testing setups. | |||
| // Use CELESTIA_PRIVATE_GENESIS env var to enable Private by specifying its genesis block hash. | |||
| Private Network = "private" | |||
| // TODO @renaynay @Wondertan: Set this once it's agreed upon, ideally this could point at a core const | |||
header/sync/sync.go
Outdated
| // ensure that head returned from trustedPeer is more recent than subjective head | ||
| // as it is possible that the trustedPeer's reported head is not current. | ||
| if objHead.Height < sbj.Height { | ||
| return nil, fmt.Errorf("trusted peer failed to return current network head") |
There was a problem hiding this comment.
return sbj head here bc it's highest we know of at that point
header/sync/sync.go
Outdated
| // return new head | ||
| return objHead, nil | ||
| } | ||
| // objHead was either ignored or rejected, return most current known |
There was a problem hiding this comment.
not ignored though as per current code, but putting IsBefore case there would make this correct
|
Implementing the |
|
Already implemented in main |
This PR extracts
Head()method into a separate interface and makesSyncerimplement it.Prerequisite for #933 to restrict the State service's access to headers to the only methods it really needs (Head).