Fix getting validators map for local relay #20
Conversation
builder/beacon_client.go
Outdated
| if nextSlotEpoch != b.currentEpoch { | ||
| nextSlotEpoch := nextSlot / b.slotsInEpoch | ||
| // if slot at half epoch, fetch next epoch's proposers | ||
| if nextSlotEpoch != b.currentEpoch || nextSlot%b.slotsInEpoch == b.slotsInEpoch/2 { |
There was a problem hiding this comment.
Thinking through this, this won't work if there is no validator registered at the mid-epoch slot (won't be called for that slot)
There was a problem hiding this comment.
we can do this the same way the boost relay does it - have a go routine that updates the validator map in the background while subscribed to the head block
There was a problem hiding this comment.
but the remote relay uses the same logic
Lines 100 to 108 in 6b30dbb
avalonche
force-pushed
the
local-relay-fix
branch
from
e8ab1e7 to
c7b1c83
Compare
builder/beacon_client.go
Outdated
| b.mu.Lock() | ||
| defer b.mu.Unlock() | ||
|
|
||
| if b.currentSlot+1 != requestedSlot { | ||
| return PubkeyHex(""), errors.New("slot out of sync") | ||
| if (requestedSlot / b.slotsInEpoch) > b.currentEpoch { |
There was a problem hiding this comment.
Hm, since b.currentEpoch is set elsewhere, calling this function twice with the same requestedSlot (which is possible) causes currEpochProposerMap to be cycled twice which might be incorrect
There was a problem hiding this comment.
don't really understand how it'll be cycled twice if b.currentEpoch is strictly increasing. Could you give an example?
There was a problem hiding this comment.
Call it twice in a row with the same requestedSlot and the asignment to currEpochProposerMap will happen twice as currentEpoch is not set in this function
builder/beacon_client.go
Outdated
| timer := time.NewTicker(durationPerEpoch / 2) | ||
| for { | ||
| select { | ||
| case <-timer.C: |
There was a problem hiding this comment.
Does the ticker tick straight away? To trigger it right away (and not wait) you could set it to zero initially.
Or factor out the update routine to a function and call it before the ticker loop for initial setup (and it's okay to wait if it fails to connect).
builder/beacon_client.go
Outdated
| } | ||
|
|
||
| currentEpoch := currentSlot / b.slotsInEpoch | ||
| if currentEpoch > b.currentEpoch { |
There was a problem hiding this comment.
There's actually a race for b.currentEpoch between this loop and onForkchoiceUpdate
I wonder if just keeping a most up to date slot proposer map instead of two copies would solve most if not all of those issues.
The slot proposer map contains slots from this and the next epochs, so you have a whole epoch of runway to update it (if that fails, there are bigger issues than the builder).
Instead of keeping nextEpochProposerMap and currEpochProposerMap you could just keep currEpochProposerMap up to date in this loop.
There was a problem hiding this comment.
onForkchoiceUpdate is not used anymore, hence the PR. We should remove it.
avalonche
force-pushed
the
local-relay-fix
branch
from
74243ea to
3747c16
Compare
avalonche
force-pushed
the
local-relay-fix
branch
from
3747c16 to
1f2047d
Compare
* Fix getting validators map for local relay * pr comments * add timer for updating known validators * improvement to local validator map fetching * lock for map updating * properly lock updates * get current slot if the mapping is empty * remove onForkchoiceUpdate * graceful shutdown * Split initial proposer sync from the proposer fetch loop (#28) Co-authored-by: Mateusz Morusiewicz <[email protected]>
* Fix getting validators map for local relay * pr comments * add timer for updating known validators * improvement to local validator map fetching * lock for map updating * properly lock updates * get current slot if the mapping is empty * remove onForkchoiceUpdate * graceful shutdown * Split initial proposer sync from the proposer fetch loop (#28) Co-authored-by: Mateusz Morusiewicz <[email protected]>
* Fix getting validators map for local relay * pr comments * add timer for updating known validators * improvement to local validator map fetching * lock for map updating * properly lock updates * get current slot if the mapping is empty * remove onForkchoiceUpdate * graceful shutdown * Split initial proposer sync from the proposer fetch loop (#28) Co-authored-by: Mateusz Morusiewicz <[email protected]>
* Fix getting validators map for local relay * pr comments * add timer for updating known validators * improvement to local validator map fetching * lock for map updating * properly lock updates * get current slot if the mapping is empty * remove onForkchoiceUpdate * graceful shutdown * Split initial proposer sync from the proposer fetch loop (#28) Co-authored-by: Mateusz Morusiewicz <[email protected]>
* Fix getting validators map for local relay * pr comments * add timer for updating known validators * improvement to local validator map fetching * lock for map updating * properly lock updates * get current slot if the mapping is empty * remove onForkchoiceUpdate * graceful shutdown * Split initial proposer sync from the proposer fetch loop (#28) Co-authored-by: Mateusz Morusiewicz <[email protected]>
* Fix getting validators map for local relay * pr comments * add timer for updating known validators * improvement to local validator map fetching * lock for map updating * properly lock updates * get current slot if the mapping is empty * remove onForkchoiceUpdate * graceful shutdown * Split initial proposer sync from the proposer fetch loop (#28) Co-authored-by: Mateusz Morusiewicz <[email protected]>
📝 Summary
Previously the local relay used the onForkChoice hook to update the validator mapping. However now that the hook is unused, the builder does not update the mapping when the local relay option is enabled.
📚 References
CONTRIBUTING.md