Cache the start key for message pruning and use latest confirmed #1757
Conversation
cla-bot
bot
added
the
s
| @@ -434,6 +434,10 @@ func (s *HeaderReader) Client() arbutil.L1Interface { | |||
| return s.client | |||
| } | |||
|
|
|||
| func (s *HeaderReader) UseFinalityData() bool { | |||
There was a problem hiding this comment.
Unless I'm missing something.. Seems like callopts in both l1Validator and Staker are initialized without params.
What if HeaderReader instead implemented a "getcallopts" function, that would return callopts which request the appropriate block?
This could simplify some things.
There was a problem hiding this comment.
could have a param to such function that optionally ignores the treatfinality if in some cases you always want latest.
There was a problem hiding this comment.
I think we'll just want to wait until the new challenge protocol replaces a lot of this code in this case
staker/staker.go
Outdated
| @@ -280,8 +289,42 @@ func (s *Staker) Initialize(ctx context.Context) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (s *Staker) latestNodeDetailsForUpdate(ctx context.Context, description string, node uint64) (arbutil.MessageIndex, *validator.GoGlobalState, error) { | |||
| stakedInfo, err := s.rollup.LookupNode(ctx, node) | |||
There was a problem hiding this comment.
you could merge more code by using the fact that LatestConfirmed is identical to LatestStaked when getting zeros for address.
Could even test if walletOrZero is zeroes and in that case skip checking latestStaked (instead - use what you have for latestConfirmed)
There was a problem hiding this comment.
I've unified the functions. Let me know what you think. I spent a while thinking about how to do this and my current approach is the best I came up with, but there might be a slightly cleaner way.
There was a problem hiding this comment.
One thing I didn't want to do is have LatestStakedNotifier and LatestConfirmedNotifier be the same interface, because I could imagine some components wanting both, and I don't want them to be confusable (looking at a component it should be clear whether it gets the latest staked or confirmed info).
While shutting down my node, I noticed it would often be taking a while in the message pruner:
It would appear leveldb iteration isn't as efficient as I thought it would be. Perhaps the fact that we'd just deleted a lot of keys in the db that we'd otherwise be iterating over is slowing it down. Regardless, I've added in this PR an in-memory cache of where we should start pruning to avoid paying this cost repeatedly. This shouldn't pose an issue where we miss a key that was re-inserted, because we're only pruning confirmed messages which are at least a week old and can't be reorg'd.
I added a second commit to this PR (I'd recommend reviewing them individually) that switches (back?) to using the latest confirmed instead of latest staked rollup node for message pruning. We can't prune what we're staked on, because we might get a challenge about something before that, but we can't get a challenge about something already confirmed so we're safe to prune there.