Switch to publishBlockV2 endpoint #492
Conversation
avalonche
force-pushed
the
publish-block-v2
branch
from
da57c0f to
c3a1829
Compare
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #492 +/- ##
==========================================
- Coverage 30.67% 30.53% -0.15%
==========================================
Files 24 24
Lines 4815 4841 +26
==========================================
+ Hits 1477 1478 +1
- Misses 3140 3164 +24
- Partials 198 199 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| func (c *ProdBeaconInstance) PublishBlock(block *common.SignedBeaconBlock, broadcastValidation BroadcastValidation) (code int, err error) { | ||
| uri := fmt.Sprintf("%s/eth/v2/beacon/blocks?broadcast_validation=%s", c.beaconURI, broadcastValidation.String()) | ||
| headers := http.Header{} | ||
| headers.Add("Eth-Consensus-Version", common.ForkVersionStringCapella) |
There was a problem hiding this comment.
Will this work with deneb? Could we un-hardcode this?
There was a problem hiding this comment.
yes, as part of #489 planning to switch over to a VersionedSignedBeaconBlock in order to get the consensus version
| ) | ||
|
|
||
| func (b BroadcastValidation) String() string { | ||
| return [...]string{"gossip", "consensus", "consensus_and_equivocation"}[b] |
There was a problem hiding this comment.
Minor nit: technically BroadcastValidation is just an int and could be any int value, which wouldn't work so well here. But I don't think it's worth changing though, as parseBroadcastValidationString is safe & you'd have to intentionally make this mistake.
There was a problem hiding this comment.
I think a simple if int(b) >= len(broadcastValidationStrings) { will suffice, will add as part of future PR
| @@ -19,7 +20,17 @@ var ( | |||
| StateIDJustified = "justified" | |||
| ) | |||
|
|
|||
| func fetchBeacon(method, url string, payload, dst any, timeout *time.Duration) (code int, err error) { | |||
| func parseBroadcastValidationString(s string) (BroadcastValidation, bool) { | |||
There was a problem hiding this comment.
comment about what this function does would be nice. also a test
| @@ -80,6 +94,18 @@ func NewMultiBeaconClient(log *logrus.Entry, beaconInstances []IBeaconInstance) | |||
| client.ffAllowSyncingBeaconNode = true | |||
| } | |||
|
|
|||
| if os.Getenv("BROADCAST_VALIDATION") != "" { | |||
There was a problem hiding this comment.
document!
also don't call os.Getenv twice, rather once and then work on the string
| @@ -64,6 +76,7 @@ type MultiBeaconClient struct { | |||
|
|
|||
| // feature flags | |||
| ffAllowSyncingBeaconNode bool | |||
| ffBroadcastValidation BroadcastValidation | |||
There was a problem hiding this comment.
ff prefix means feature flag, usually bool
this should probably be broadcastMode
|
Note: these changes may or may not work with current CL clients. Currently testing with:
|
📝 Summary
Switching calling
publishBlockV1topublishBlockV2in the relay. Default broadcast validation isconsensus_and_equivocationand can be toggled using env vars.⛱ Motivation and Context
The new
publishBlockV2endpoint have extra parameters to allow consensus and equivocation checks. This is already implemented in prysm and lighthouse and will be needed for the Deneb upgrades.📚 References
publishBlockV2endpoint: https://ethereum.github.io/beacon-APIs/#/ValidatorRequiredApi/publishBlockV2✅ I have run these commands
make lintmake test-racego mod tidyCONTRIBUTING.md