Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

geth: Sync status always %100. #2017

Closed
JoeGruffins opened this issue Dec 30, 2022 · 10 comments · Fixed by #2034
Closed

geth: Sync status always %100. #2017

JoeGruffins opened this issue Dec 30, 2022 · 10 comments · Fixed by #2034
Labels
bug bug or bugfix ETH
Milestone

Comments

@JoeGruffins
Copy link
Member

Server will start the markets before chain is synced. Client just says %100 and will trade when it should no be able to yet.

@chappjc
Copy link
Member

chappjc commented Dec 31, 2022

Between this and #2016, the authrpc interface seems to be trouble. I think we should seek clarification on what can we can expect from the authrpc interface, namely, can we use it just like the http/ws interfaces, just with authentication (and the engine api module that only the consensus client uses).

@chappjc
Copy link
Member

chappjc commented Dec 31, 2022

Oh wait, we're intentionally doing this:

// syncProgress: We're going to lie and just always say we're synced if we
// can get a header.
func (m *multiRPCClient) syncProgress(ctx context.Context) (prog *ethereum.SyncProgress, err error) {
return prog, m.withAny(func(p *provider) error {
tip, err := p.bestHeader(ctx, m.log)
if err != nil {
return err
}
prog = &ethereum.SyncProgress{
CurrentBlock: tip.Number.Uint64(),
HighestBlock: tip.Number.Uint64(),

HighestBlock is always equal to CurrentBlock if we can get a header. Clearly we have to do something smarter than that.

@JoeGruffins
Copy link
Member Author

I knew it was intentional with the client, but not sure with the server. Will look into it.

@JoeGruffins
Copy link
Member Author

This seems to be the case with server over ipc as well.

@chappjc chappjc added this to the 0.6 milestone Jan 2, 2023
@chappjc chappjc added ETH bug bug or bugfix labels Jan 4, 2023
@JoeGruffins JoeGruffins changed the title geth: Sync status always %100 over websocket with jwt. geth: Sync status always %100. Jan 5, 2023
@JoeGruffins
Copy link
Member Author

Yes, ipc for client to. Easy to observe on testnet with a new eth node combo. I guess something has changed with sync status since the merge.

@JoeGruffins
Copy link
Member Author

Putting in some logs with client over ipc, SyncProgress does not seem to be helpful at all anymore:

(*ethereum.SyncProgress)(0xc007440a20)({
 StartingBlock: (uint64) 0,
 CurrentBlock: (uint64) 8158986,
 HighestBlock: (uint64) 8158986,
 PulledStates: (uint64) 0,
 KnownStates: (uint64) 0,
 SyncedAccounts: (uint64) 0,
 SyncedAccountBytes: (uint64) 0,
 SyncedBytecodes: (uint64) 0,
 SyncedBytecodeBytes: (uint64) 0,
 SyncedStorage: (uint64) 0,
 SyncedStorageBytes: (uint64) 0,
 HealedTrienodes: (uint64) 0,
 HealedTrienodeBytes: (uint64) 0,
 HealedBytecodes: (uint64) 0,
 HealedBytecodeBytes: (uint64) 0,
 HealingTrienodes: (uint64) 0,
 HealingBytecode: (uint64) 0
})
2023-01-06 06:44:13.880 [DBG] CORE[eth][ETH]: tip change: 8158986 (0x80127920c3f3eb82c5fe5c27c120e7210f7fad5f0bc6b2b9f839a21f2ff0fb8e) => 8159105 (0x9cc6675f787edd3dc7be18dc5532c6595e1ac0daefb2710dfb87c3defb992aeb)
(*ethereum.SyncProgress)(0xc007440b40)({
 StartingBlock: (uint64) 0,
 CurrentBlock: (uint64) 8159105,
 HighestBlock: (uint64) 8159105,
 PulledStates: (uint64) 0,
 KnownStates: (uint64) 0,
 SyncedAccounts: (uint64) 0,
 SyncedAccountBytes: (uint64) 0,
 SyncedBytecodes: (uint64) 0,
 SyncedBytecodeBytes: (uint64) 0,
 SyncedStorage: (uint64) 0,
 SyncedStorageBytes: (uint64) 0,
 HealedTrienodes: (uint64) 0,
 HealedTrienodeBytes: (uint64) 0,
 HealedBytecodes: (uint64) 0,
 HealedBytecodeBytes: (uint64) 0,
 HealingTrienodes: (uint64) 0,
 HealingBytecode: (uint64) 0
})

While the node acknowledges we are a few weeks behind:

INFO [01-06|15:46:18.062] Imported new chain segment               blocks=103 txs=5739 mgas=1144.034 elapsed=8.002s    mgasps=142.951 number=8,161,194 hash=010435..e6afbc age=2w4d2h   dirty=255.31MiB

@JoeGruffins
Copy link
Member Author

Looking around for some info, maybe we have to fall back to header age again.

@JoeGruffins
Copy link
Member Author

JoeGruffins commented Jan 6, 2023

The server is checking header age, but the calculation is wrong. Probably my fault.

@JoeGruffins
Copy link
Member Author

JoeGruffins commented Jan 6, 2023

Unrelated, but my goerli geth node is filled with this error:

ERROR[01-06|16:53:27.270] Expired request does not exist           peer=6d22e6f09a020aeb80a463ed794a0e2ef6402d131723a3b3ceb90abb634b2739
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 
ERROR[01-06|16:53:27.906] Unhandled trie error: missing trie node 5d12e74e5231fb1a3e2bfe2d216f2dcf4a6cebed3ff1c83ae8611ea135040fe9 (path 08) <nil> 

@chappjc
Copy link
Member

chappjc commented Jan 10, 2023

Yes, ipc for client to. Easy to observe on testnet with a new eth node combo. I guess something has changed with sync status since the merge.

ethereum/go-ethereum#25534 (comment)

Post merge there's a weird new quirk. If the beacon client tells us "hey, sync your chain to this new HEAD", then we will spin up a new sync cycle and do all the usual reportings. However, what usually happens is that the beacon client retrieves the (beacon) blocks 1 by 1, unpacks the payload and feeds it to us, 1 by 1. In that case, we're never really syncing, we're just receiving 1 new block each time, which we import.

Perhaps we could (after the merge) visit this issue and extend the newpayload API to feed some more metadata from the beacon client to the execution so we might also know what's happening.

So I agree it looks like we need to look at header age.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bug or bugfix ETH
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants