Skip to content

Commit

Permalink
app/obolapi: ignore empty partial signatures (#50)
Browse files Browse the repository at this point in the history
API will return partial signatures iff there are at least threshold of them, which also means we could find `null`'s in the response.

Since the partial signatures array returned by the API maps 1:1 with the corresponding share index, we need to ignore empty ones to keep going with the aggregation process.

We still error if the signature is longer than 0 bytes but no more than 2.

Added a test case to cover the issue as well.
  • Loading branch information
gsora authored Jan 12, 2024
1 parent 3d5c86a commit 20d28fd
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 8 deletions.
5 changes: 5 additions & 0 deletions app/obolapi/obolapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@ func (c Client) GetFullExit(ctx context.Context, valPubkey string, lockHash []by
rawSignatures := make(map[int]tbls.Signature)

for sigIdx, sigStr := range er.Signatures {
if len(sigStr) == 0 {
// ignore, the associated share index didn't push a partial signature yet
continue
}

if len(sigStr) < 2 {
return ExitBlob{}, errors.New("signature string has invalid size", z.Int("size", len(sigStr)))
}
Expand Down
86 changes: 85 additions & 1 deletion app/obolapi/obolapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const exitEpoch = eth2p0.Epoch(162304)
func TestAPIFlow(t *testing.T) {
kn := 4

handler, addLockFiles := obolapi.MockServer()
handler, addLockFiles := obolapi.MockServer(false)
srv := httptest.NewServer(handler)

defer srv.Close()
Expand Down Expand Up @@ -108,6 +108,90 @@ func TestAPIFlow(t *testing.T) {
}
}

func TestAPIFlowMissingSig(t *testing.T) {
kn := 4

handler, addLockFiles := obolapi.MockServer(true)
srv := httptest.NewServer(handler)

defer srv.Close()

bnapiHandler := bnapi.MockBeaconNode(nil)
bnapiServer := httptest.NewServer(bnapiHandler)
defer bnapiServer.Close()
mockEth2Cl := eth2Client(t, context.Background(), bnapiServer.URL)

lock, identityKeys, shares := cluster.NewForT(
t,
1,
kn-1,
kn,
0,
)

addLockFiles(lock)

exitMsg := eth2p0.SignedVoluntaryExit{
Message: &eth2p0.VoluntaryExit{
Epoch: 42,
ValidatorIndex: 42,
},
Signature: eth2p0.BLSSignature{},
}

sigRoot, err := exitMsg.Message.HashTreeRoot()
require.NoError(t, err)

domain, err := signing.GetDomain(context.Background(), mockEth2Cl, signing.DomainExit, exitEpoch)
require.NoError(t, err)

sigData, err := (&eth2p0.SigningData{ObjectRoot: sigRoot, Domain: domain}).HashTreeRoot()
require.NoError(t, err)

for idx := 0; idx < len(shares); idx++ {
var exits []obolapi.ExitBlob

for _, shareSet := range shares[idx] {
signature, err := tbls.Sign(shareSet, sigData[:])
require.NoError(t, err)

exitMsg := exitMsg
exitMsg.Signature = eth2p0.BLSSignature(signature)

exit := obolapi.ExitBlob{
PublicKey: lock.Validators[0].PublicKeyHex(),
SignedExitMessage: exitMsg,
}

exits = append(exits, exit)
}

cl := obolapi.Client{ObolAPIUrl: srv.URL}

ctx := context.Background()

// send all the partial exits
for idx, exit := range exits {
require.NoError(t, cl.PostPartialExit(ctx, lock.LockHash, uint64(idx+1), identityKeys[idx], exit), "share index: %d", idx+1)
}

for idx := range exits {
// get full exit
fullExit, err := cl.GetFullExit(ctx, lock.Validators[0].PublicKeyHex(), lock.LockHash, uint64(idx+1), identityKeys[idx])
require.NoError(t, err, "share index: %d", idx+1)

valPubk, err := lock.Validators[0].PublicKey()
require.NoError(t, err, "share index: %d", idx+1)

sig, err := tblsconv.SignatureFromBytes(fullExit.SignedExitMessage.Signature[:])
require.NoError(t, err, "share index: %d", idx+1)

// verify that the aggregated signature works
require.NoError(t, tbls.Verify(valPubk, sigData[:], sig), "share index: %d", idx+1)
}
}
}

func eth2Client(t *testing.T, ctx context.Context, bnURL string) eth2wrap.Client {
t.Helper()

Expand Down
14 changes: 11 additions & 3 deletions app/obolapi/test_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ type testServer struct {

// store the lock file by its lock hash
lockFiles map[string]cluster.Lock

// drop one partial signature when returning the full set
dropOnePsig bool
}

// addLockFiles adds a set of lock files to ts.
Expand Down Expand Up @@ -232,6 +235,10 @@ func (ts *testServer) HandleFullExit(writer http.ResponseWriter, request *http.R
ret.ValidatorIndex = pExit.SignedExitMessage.Message.ValidatorIndex
}

if ts.dropOnePsig {
ret.Signatures[0] = "" // blank out the first signature, as if the API didn't receive the partial exit for it
}

if err := json.NewEncoder(writer).Encode(ret); err != nil {
writeErr(writer, http.StatusInternalServerError, errors.Wrap(err, "cannot marshal exit message").Error())
return
Expand Down Expand Up @@ -276,11 +283,12 @@ func cleanTmpl(tmpl string) string {

// MockServer returns a obol API mock test server.
// It returns a http.Handler to be served over HTTP, and a function to add cluster lock files to its database.
func MockServer() (http.Handler, func(lock cluster.Lock)) {
func MockServer(dropOnePsig bool) (http.Handler, func(lock cluster.Lock)) {
ts := testServer{
lock: sync.Mutex{},
partialExits: map[string][]exitBlob{},
lockFiles: map[string]cluster.Lock{},
dropOnePsig: dropOnePsig,
}

router := mux.NewRouter()
Expand All @@ -295,8 +303,8 @@ func MockServer() (http.Handler, func(lock cluster.Lock)) {
}

// Run runs obol api mock on the provided bind port.
func Run(_ context.Context, bind string, locks []cluster.Lock) error {
ms, addLock := MockServer()
func Run(_ context.Context, bind string, locks []cluster.Lock, dropOnePsig bool) error {
ms, addLock := MockServer(dropOnePsig)

for _, lock := range locks {
addLock(lock)
Expand Down
2 changes: 1 addition & 1 deletion app/util/testutil/api_servers.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (ts *TestServers) Eth2Client(t *testing.T, ctx context.Context) eth2wrap.Cl
func APIServers(t *testing.T, lock cluster.Lock, withNonActiveVals bool) TestServers {
t.Helper()

oapiHandler, oapiAddLock := obolapi.MockServer()
oapiHandler, oapiAddLock := obolapi.MockServer(false)
oapiAddLock(lock)

oapiServer := httptest.NewServer(oapiHandler)
Expand Down
6 changes: 3 additions & 3 deletions cmd/mockservers.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type bmockCliConfig struct {
func newMockServersCmd(
root *cobra.Command,
bnapiMock func(ctx context.Context, validators map[string]eth2v1.Validator, bindAddr string) error,
obolAPIMock func(_ context.Context, bind string, locks []cluster.Lock) error,
obolAPIMock func(_ context.Context, bind string, locks []cluster.Lock, _ bool) error,
) {
bcc := bmockCliConfig{
Validators: map[string]eth2v1.Validator{},
Expand Down Expand Up @@ -141,7 +141,7 @@ func runMockServers(
cmd *cobra.Command,
conf bmockCliConfig,
bnapiMock func(ctx context.Context, validators map[string]eth2v1.Validator, bindAddr string) error,
obolAPIMock func(_ context.Context, bind string, locks []cluster.Lock) error,
obolAPIMock func(_ context.Context, bind string, locks []cluster.Lock, _ bool) error,
) error {
if err := log.InitLogger(conf.Log); err != nil {
return err
Expand All @@ -158,7 +158,7 @@ func runMockServers(
})

eg.Go(func() error {
return obolAPIMock(ctx, conf.ObolAPIBind, conf.LockFiles)
return obolAPIMock(ctx, conf.ObolAPIBind, conf.LockFiles, false)
})

if err := eg.Wait(); err != nil {
Expand Down

0 comments on commit 20d28fd

Please sign in to comment.