Client: More UI Improvements / Fix TxPool not being started along FCU #3100
Conversation
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -1226,11 +1226,9 @@ export class Engine { | |||
| } | |||
| this.service.txPool.removeNewBlockTxs(blocks) | |||
|
|
|||
| const isPrevSynced = this.chain.config.synchronized | |||
| this.config.updateSynchronizedState(headBlock.header) | |||
There was a problem hiding this comment.
Should this line be removed? 🤔
There was a problem hiding this comment.
It's was not triggering anything anyhow and is now replaced by a direct setting of synchronized on FCU.
Testing eventual side effects right now by running a full Holesky sync (seems nothing so far).
Point of all this is that the tx pool never started.
There was a problem hiding this comment.
I don't think we should remove this line. The txpool thing is fine. Now each time FCU is called it will set state to synchronized. There is an interval in sync.ts which calls it also, which might override it back to syncing. I think this has undesired side effects..?
There was a problem hiding this comment.
I guess i"m not 100% clear on what this synchronized state represents then. Does it represent the current head that the CL knows of or is it the current canonical head that the client is aware of or yet again, is it the current executed head of the client?
cc @g11tech
There was a problem hiding this comment.
We generally have messy "synchronized" setting logic, especially in a post-Merge context.
This updateSynchronizedState() method e.g. - post-Mege - didn't trigger at all any more (didn't analyze further), with one (likely beyond other) before-change side effects that the tx pool did not start.
I now updated based on the assumed definition that the chain is synchronized "by definition" after a succesfull FCU including execution.
Side note: that doesn't mean our synchronized-code is completely clean after that. We should take this upon on a separate occasion and generally clean this up and make this more consistent.
There was a problem hiding this comment.
i will lookinto fhis and make a fix shortly, as discussed with @holgerd77 the new logic might open the txpool without us being in the "synchronized" state
|
Ah, no side effects (ran full Holesky and tx pool is properly starting at the end of execution and no other "stuff" happening or triggered).
Then I guess this is ready for review! 🙂 |
No description provided.