-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: voluntary exit #648
core: voluntary exit #648
Changes from 1 commit
18a44ae
45855db
6fc95da
bc2c1d1
9ac9a3e
a6ef600
ee1b6df
6f5771f
1803159
3e7f17d
cec382c
04f9af5
e8b3a5c
2b291fe
04333a0
a3ea88a
6591fb3
a6ed288
e400958
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ package bcast | |
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
|
||
eth2client "github.com/attestantio/go-eth2-client" | ||
eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0" | ||
|
@@ -32,6 +33,7 @@ import ( | |
type eth2Provider interface { | ||
eth2client.AttestationsSubmitter | ||
eth2client.BeaconBlockSubmitter | ||
eth2client.VoluntaryExitSubmitter | ||
} | ||
|
||
// New returns a new broadcaster instance. | ||
|
@@ -93,6 +95,24 @@ func (b Broadcaster) Broadcast(ctx context.Context, duty core.Duty, | |
case core.DutyRandao: | ||
// Randao is an internal duty, not broadcasted to beacon chain | ||
return nil | ||
case core.DutyVoluntaryExit: | ||
// JSON decoding from the previous component | ||
ve := new(eth2p0.SignedVoluntaryExit) | ||
err := json.Unmarshal(aggData.Data, ve) | ||
if err != nil { | ||
return errors.Wrap(err, "json decoding voluntary exit") | ||
} | ||
|
||
err = b.eth2Cl.SubmitVoluntaryExit(ctx, ve) | ||
|
||
if err == nil { | ||
log.Info(ctx, "Voluntary exit successfully submitted to beacon node", | ||
z.U64("epoch", uint64(ve.Message.Epoch)), | ||
z.U64("validatorIndex", uint64(ve.Message.ValidatorIndex)), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use snake case for json logging fields (see guidelines) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would also add the pubkey here: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
) | ||
} | ||
|
||
return err | ||
default: | ||
return errors.New("unsupported duty type") | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ package bcast_test | |
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
"testing" | ||
|
||
"github.com/attestantio/go-eth2-client/spec" | ||
|
@@ -86,3 +87,40 @@ func TestBroadcastBeaconBlock(t *testing.T) { | |
|
||
<-ctx.Done() | ||
} | ||
|
||
func TestBroadcastVoluntaryExit(t *testing.T) { | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
mock, err := beaconmock.New() | ||
require.NoError(t, err) | ||
|
||
ve := ð2p0.SignedVoluntaryExit{ | ||
Message: ð2p0.VoluntaryExit{ | ||
Epoch: 10, | ||
ValidatorIndex: 10, | ||
}, | ||
Signature: testutil.RandomEth2Signature(), | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could extract this as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think it is too small to extract plus it would be used only here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
aggDataData, err := json.Marshal(ve) | ||
require.NoError(t, err) | ||
|
||
aggData := core.AggSignedData{ | ||
Data: aggDataData, | ||
Signature: core.SigFromETH2(ve.Signature), | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extract this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
mock.SubmitVoluntaryExitFunc = func(ctx context.Context, ve2 *eth2p0.SignedVoluntaryExit) error { | ||
require.Equal(t, ve, ve2) | ||
cancel() | ||
|
||
return ctx.Err() | ||
} | ||
|
||
bcaster, err := bcast.New(mock) | ||
require.NoError(t, err) | ||
|
||
err = bcaster.Broadcast(ctx, core.Duty{Type: core.DutyVoluntaryExit}, "", aggData) | ||
require.ErrorIs(t, err, context.Canceled) | ||
|
||
<-ctx.Done() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ package sigagg | |
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
|
||
"github.com/attestantio/go-eth2-client/spec" | ||
eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0" | ||
|
@@ -148,6 +149,27 @@ func getAggSignedData(typ core.DutyType, data core.ParSignedData, aggSig *bls_si | |
} | ||
|
||
return core.EncodeBlockAggSignedData(block) | ||
case core.DutyVoluntaryExit: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// JSON decode from previous component | ||
ve := new(eth2p0.SignedVoluntaryExit) | ||
err := json.Unmarshal(data.Data, ve) | ||
if err != nil { | ||
return core.AggSignedData{}, errors.Wrap(err, "json decoding voluntary exit") | ||
} | ||
|
||
// change signature to TSS aggregated one | ||
ve.Signature = eth2Sig | ||
|
||
// JSON encode for next component | ||
data, err := json.Marshal(ve) | ||
if err != nil { | ||
return core.AggSignedData{}, errors.Wrap(err, "json encoding voluntary exit") | ||
} | ||
|
||
return core.AggSignedData{ | ||
Data: data, | ||
Signature: core.SigFromETH2(eth2Sig), | ||
}, nil | ||
default: | ||
return core.AggSignedData{}, errors.New("unsupported duty type") | ||
} | ||
|
@@ -178,6 +200,15 @@ func getSignedRoot(typ core.DutyType, data core.ParSignedData) (eth2p0.Root, err | |
} | ||
|
||
return block.Root() | ||
case core.DutyVoluntaryExit: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// JSON decode from previous component | ||
ve := new(eth2p0.SignedVoluntaryExit) | ||
err := json.Unmarshal(data.Data, ve) | ||
if err != nil { | ||
return eth2p0.Root{}, errors.Wrap(err, "json decoding voluntary exit") | ||
} | ||
|
||
return ve.Message.HashTreeRoot() | ||
default: | ||
return eth2p0.Root{}, errors.New("unsupported duty type") | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,13 +32,14 @@ type DutyType int | |
const ( | ||
// DutyType enums MUST not change, it will break backwards compatibility. | ||
|
||
DutyUnknown DutyType = 0 | ||
DutyProposer DutyType = 1 | ||
DutyAttester DutyType = 2 | ||
DutyRandao DutyType = 3 | ||
DutyUnknown DutyType = 0 | ||
DutyProposer DutyType = 1 | ||
DutyAttester DutyType = 2 | ||
DutyRandao DutyType = 3 | ||
DutyVoluntaryExit DutyType = 4 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would propose we rename this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't fell comfortable with this, there will be other Exits in the code, could we ask for feedback to @OisinKyne @dB2510 @xenowits ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// Only ever append new types here... | ||
|
||
dutySentinel DutyType = 4 // Must always be last | ||
dutySentinel DutyType = 5 // Must always be last | ||
) | ||
|
||
func (d DutyType) Valid() bool { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,7 @@ type Handler interface { | |
eth2client.BeaconBlockSubmitter | ||
eth2client.ProposerDutiesProvider | ||
eth2client.ValidatorsProvider | ||
eth2client.VoluntaryExitSubmitter | ||
// Above sorted alphabetically. | ||
} | ||
|
||
|
@@ -71,6 +72,7 @@ func NewRouter(h Handler, beaconNodeAddr string) (*mux.Router, error) { | |
Name string | ||
Path string | ||
Handler handlerFunc | ||
Methods []string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure this is required, why not just support any method since it doesn't really matter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we want to do this, we should probably be consistent and add methods for all endpoints (in a different PR) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, we don't need methods since we use eth2-http package for handler which internally handles methods and stuff. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We only want to capture the POST, the GET should be proxied |
||
}{ | ||
{ | ||
Name: "attester_duties", | ||
|
@@ -112,12 +114,21 @@ func NewRouter(h Handler, beaconNodeAddr string) (*mux.Router, error) { | |
Path: "/eth/v1/beacon/blocks", | ||
Handler: submitBlock(h), | ||
}, | ||
{ | ||
Name: "submit_voluntary_exit", | ||
Path: "/eth/v1/beacon/pool/voluntary_exits", | ||
Handler: submitVoluntaryExit(h), | ||
Methods: []string{"POST"}, | ||
}, | ||
// TODO(corver): Add more endpoints | ||
} | ||
|
||
r := mux.NewRouter() | ||
for _, e := range endpoints { | ||
r.Handle(e.Path, wrap(e.Name, e.Handler)) | ||
route := r.Handle(e.Path, wrap(e.Name, e.Handler)) | ||
if e.Methods != nil && len(e.Methods) > 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if statement not required
|
||
route.Methods(e.Methods...) | ||
} | ||
} | ||
|
||
// Everything else is proxied | ||
|
@@ -439,6 +450,19 @@ func submitBlock(p eth2client.BeaconBlockSubmitter) handlerFunc { | |
} | ||
} | ||
|
||
// submitVoluntaryExit receives the partially signed voluntary exit. | ||
func submitVoluntaryExit(p eth2client.VoluntaryExitSubmitter) handlerFunc { | ||
return func(ctx context.Context, params map[string]string, query url.Values, body []byte) (interface{}, error) { | ||
ve := new(eth2p0.SignedVoluntaryExit) | ||
err := ve.UnmarshalJSON(body) | ||
if err != nil { | ||
return nil, errors.New("invalid voluntary exit data") | ||
} | ||
|
||
return nil, p.SubmitVoluntaryExit(ctx, ve) | ||
} | ||
} | ||
|
||
// proxyHandler returns a reverse proxy handler. | ||
func proxyHandler(target string) (http.HandlerFunc, error) { | ||
targetURL, err := url.Parse(target) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -457,6 +457,29 @@ func TestRouter(t *testing.T) { | |
|
||
testRouter(t, handler, callback) | ||
}) | ||
|
||
t.Run("submit_voluntary_exit", func(t *testing.T) { | ||
ve := ð2p0.SignedVoluntaryExit{ | ||
Message: ð2p0.VoluntaryExit{ | ||
Epoch: 10, | ||
ValidatorIndex: 10, | ||
}, | ||
Signature: testutil.RandomEth2Signature(), | ||
} | ||
|
||
handler := testHandler{ | ||
SubmitVoluntaryExitFunc: func(ctx context.Context, exit *eth2p0.SignedVoluntaryExit) error { | ||
return nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should probably assert the data is correct |
||
}, | ||
} | ||
|
||
callback := func(ctx context.Context, cl *eth2http.Service) { | ||
err := cl.SubmitVoluntaryExit(ctx, ve) | ||
require.NoError(t, err) | ||
} | ||
|
||
testRouter(t, handler, callback) | ||
}) | ||
} | ||
|
||
// testRouter is a helper function to test router endpoints with an eth2http client. The outer test | ||
|
@@ -509,6 +532,7 @@ type testHandler struct { | |
ProposerDutiesFunc func(ctx context.Context, epoch eth2p0.Epoch, il []eth2p0.ValidatorIndex) ([]*eth2v1.ProposerDuty, error) | ||
ValidatorsFunc func(ctx context.Context, stateID string, indices []eth2p0.ValidatorIndex) (map[eth2p0.ValidatorIndex]*eth2v1.Validator, error) | ||
ValidatorsByPubKeyFunc func(ctx context.Context, stateID string, pubkeys []eth2p0.BLSPubKey) (map[eth2p0.ValidatorIndex]*eth2v1.Validator, error) | ||
SubmitVoluntaryExitFunc func(ctx context.Context, exit *eth2p0.SignedVoluntaryExit) error | ||
} | ||
|
||
func (h testHandler) AttestationData(ctx context.Context, slot eth2p0.Slot, commIdx eth2p0.CommitteeIndex) (*eth2p0.AttestationData, error) { | ||
|
@@ -539,6 +563,10 @@ func (h testHandler) ProposerDuties(ctx context.Context, epoch eth2p0.Epoch, il | |
return h.ProposerDutiesFunc(ctx, epoch, il) | ||
} | ||
|
||
func (h testHandler) SubmitVoluntaryExit(ctx context.Context, exit *eth2p0.SignedVoluntaryExit) error { | ||
return h.SubmitVoluntaryExitFunc(ctx, exit) | ||
} | ||
|
||
// newBeaconHandler returns a mock beacon node handler. It registers a few mock handlers required by the | ||
// eth2http service on startup, all other requests are routed to ProxyHandler if not nil. | ||
func (h testHandler) newBeaconHandler(t *testing.T) http.Handler { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow above pattern of extracting this to
core.DecodeExitAggSignedData
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done