p2p/sentry: StatusDataProvider ReadCurrentHeader error #9890
Conversation
core/rawdb/accessors_chain.go
Outdated
| @@ -284,6 +288,16 @@ func ReadCurrentHeader(db kv.Getter) *types.Header { | |||
| return ReadHeader(db, headHash, *headNumber) | |||
| } | |||
|
|
|||
| // ReadCurrentHeader2 reads the current canonical head header. | |||
| func ReadCurrentHeader2(db kv.Getter) *types.Header { | |||
There was a problem hiding this comment.
rename to ReadCurrentBlockHeader?
There was a problem hiding this comment.
I'd like to understand the difference between the 2 tables before renaming. I've asked on discord.
p2p/sentry/status_data_provider.go
Outdated
| @@ -76,6 +76,9 @@ func (s *StatusDataProvider) GetStatusData(ctx context.Context) (*proto_sentry.S | |||
|
|
|||
| func ReadChainHeadWithTx(tx kv.Tx) (ChainHead, error) { | |||
| header := rawdb.ReadCurrentHeader(tx) | |||
| if header == nil { | |||
| header = rawdb.ReadCurrentHeader2(tx) | |||
There was a problem hiding this comment.
can we just call this directly instead of calling ReadCurrentHeader first? what matters to us is the which is the last block that has been sync-ed and get its header
There was a problem hiding this comment.
The only reason I originally did the patch like this was that I found the first call was failing and like Daniel I don't know what the difference is between the data in the tables.
Once this is merged I'll overwrite my local fix before completing my PR.
There was a problem hiding this comment.
I've removed the ReadCurrentHeader call. One problem with it is that during initial sync (which can take a week) we will report to P2P that we are still on genesis, because HeadBlockKey is only updated at stage_finish. I think that conceptually it is correct (that we only report confirmed state blocks), but it is not nice for the P2P (because we are leeching for many days).
There was a problem hiding this comment.
Ideally we should report the last committed body. I'll make a follow-up with this later.
There was a problem hiding this comment.
@mh0lt I will merge it now, because it fixes a production issue. I can help to rebase your PR and fix conflicts.
p2p/sentry/status_data_provider.go
Outdated
| @@ -76,6 +76,9 @@ func (s *StatusDataProvider) GetStatusData(ctx context.Context) (*proto_sentry.S | |||
|
|
|||
| func ReadChainHeadWithTx(tx kv.Tx) (ChainHead, error) { | |||
| header := rawdb.ReadCurrentHeader(tx) | |||
| if header == nil { | |||
| header = rawdb.ReadCurrentHeader2(tx) | |||
There was a problem hiding this comment.
The only reason I originally did the patch like this was that I found the first call was failing and like Daniel I don't know what the difference is between the data in the tables.
Once this is merged I'll overwrite my local fix before completing my PR.
P2P fails on restart because rawdb.ReadCurrentHeader returns a nil header. It looks like ReadHeadHeaderHash fails to find the current header hash. However the correct hash is returned by ReadHeadBlockHash.
battlmonstr
force-pushed
the
pr/sentry_status_bug
branch
from
b35c37b
to
368beeb
Compare
ghost
mentioned this pull request
P2P fails on restart because rawdb.ReadCurrentHeader returns a nil header. It looks like ReadHeadHeaderHash fails to find the current header hash. However the correct hash is returned by ReadHeadBlockHash.