Fix getting validators map for local relay by avalonche · Pull Request #20 · flashbots/builder

avalonche · 2022-12-07T16:28:09Z

📝 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

I have seen and agree to CONTRIBUTING.md

builder/beacon_client.go

builder/config.go

Ruteri · 2022-12-08T17:13:44Z

builder/beacon_client.go

-	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 {


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)

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

Hm, should work

but the remote relay uses the same logic

builder/builder/relay.go

Lines 100 to 108 in 6b30dbb

if r.lastRequestedSlot == 0 || nextSlot/32 > r.lastRequestedSlot/32 {

// Every epoch request validators map

go func() {

err := r.updateValidatorsMap(nextSlot, 1)

if err != nil {

log.Error("could not update validators map", "err", err)

}

}()

}

did we see any corner cases around the epoch change?

builder/local_relay.go

Ruteri · 2022-12-13T21:27:32Z

builder/beacon_client.go


-	if b.currentSlot+1 != requestedSlot {
-		return PubkeyHex(""), errors.New("slot out of sync")
+	if (requestedSlot / b.slotsInEpoch) > b.currentEpoch {


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

don't really understand how it'll be cycled twice if b.currentEpoch is strictly increasing. Could you give an example?

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

Ruteri · 2022-12-13T22:48:28Z

builder/beacon_client.go

+	timer := time.NewTicker(durationPerEpoch / 2)
+	for {
+		select {
+		case <-timer.C:


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).

Ruteri · 2022-12-14T11:30:37Z

builder/beacon_client.go

+			}
+
+			currentEpoch := currentSlot / b.slotsInEpoch
+			if currentEpoch > b.currentEpoch {


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.

onForkchoiceUpdate is not used anymore, hence the PR. We should remove it.

* 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 <11313015+Ruteri@users.noreply.github.com>

Ruteri reviewed Dec 8, 2022

View reviewed changes

builder/beacon_client.go Outdated Show resolved Hide resolved

builder/beacon_client.go Outdated Show resolved Hide resolved

Ruteri reviewed Dec 8, 2022

View reviewed changes

avalonche added 3 commits December 10, 2022 09:04

Fix getting validators map for local relay

d8b0426

pr comments

01c40c5

add timer for updating known validators

c7b1c83

avalonche force-pushed the local-relay-fix branch from e8ab1e7 to c7b1c83 Compare December 9, 2022 22:24

improvement to local validator map fetching

29a4167

Ruteri reviewed Dec 13, 2022

View reviewed changes

lock for map updating

710dad4

Ruteri reviewed Dec 13, 2022

View reviewed changes

builder/beacon_client.go Outdated Show resolved Hide resolved

Ruteri reviewed Dec 13, 2022

View reviewed changes

properly lock updates

693edf6

Ruteri reviewed Dec 14, 2022

View reviewed changes

get current slot if the mapping is empty

29058c6

avalonche force-pushed the local-relay-fix branch from 74243ea to 3747c16 Compare December 14, 2022 18:12

remove onForkchoiceUpdate

1f2047d

avalonche force-pushed the local-relay-fix branch from 3747c16 to 1f2047d Compare December 14, 2022 18:22

avalonche and others added 2 commits December 14, 2022 19:02

graceful shutdown

e4f7655

Split initial proposer sync from the proposer fetch loop (#28)

3782f2d

Ruteri approved these changes Dec 15, 2022

View reviewed changes

Ruteri mentioned this pull request Dec 19, 2022

Work on local devnet #30

Merged

avalonche merged commit c3c62dd into main Dec 20, 2022

avalonche pushed a commit that referenced this pull request Feb 7, 2023

Add more explicit builder flags and other QoL improvements (#20)

84a260c

avalonche pushed a commit that referenced this pull request Feb 7, 2023

Remove megabundles as they are no longer needed (#20)

cb82f07

avalonche pushed a commit that referenced this pull request Mar 9, 2023

Add more explicit builder flags and other QoL improvements (#20)

5118498

avalonche pushed a commit that referenced this pull request Mar 9, 2023

Remove megabundles as they are no longer needed (#20)

b93e268

avalonche pushed a commit that referenced this pull request Mar 15, 2023

Add more explicit builder flags and other QoL improvements (#20)

09b38bc

avalonche pushed a commit that referenced this pull request Mar 15, 2023

Remove megabundles as they are no longer needed (#20)

7d49425

avalonche pushed a commit that referenced this pull request Mar 17, 2023

Add more explicit builder flags and other QoL improvements (#20)

b91151c

avalonche pushed a commit that referenced this pull request Mar 17, 2023

Remove megabundles as they are no longer needed (#20)

3885dcd

avalonche pushed a commit that referenced this pull request Mar 22, 2023

Add more explicit builder flags and other QoL improvements (#20)

9499bee

avalonche pushed a commit that referenced this pull request Mar 22, 2023

Remove megabundles as they are no longer needed (#20)

ca7281e

avalonche pushed a commit that referenced this pull request Jul 6, 2023

Add more explicit builder flags and other QoL improvements (#20)

9ef7ab8

avalonche pushed a commit that referenced this pull request Jul 6, 2023

Remove megabundles as they are no longer needed (#20)

077db7f

	if r.lastRequestedSlot == 0 \|\| nextSlot/32 > r.lastRequestedSlot/32 {
	// Every epoch request validators map
	go func() {
	err := r.updateValidatorsMap(nextSlot, 1)
	if err != nil {
	log.Error("could not update validators map", "err", err)
	}
	}()
	}

Conversation

avalonche commented Dec 7, 2022

📝 Summary

📚 References

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants