From b2b00a3d34704882d766ff8ef890a5c15241eb7b Mon Sep 17 00:00:00 2001 From: Jian Xiao Date: Sat, 2 Mar 2024 05:30:29 +0000 Subject: [PATCH 1/7] [4/N][multi quorum ejection] Create operator quorum intervals --- disperser/dataapi/nonsigner_handlers.go | 163 ++++++++ disperser/dataapi/nonsigner_handlers_test.go | 372 +++++++++++++++++++ 2 files changed, 535 insertions(+) create mode 100644 disperser/dataapi/nonsigner_handlers.go create mode 100644 disperser/dataapi/nonsigner_handlers_test.go diff --git a/disperser/dataapi/nonsigner_handlers.go b/disperser/dataapi/nonsigner_handlers.go new file mode 100644 index 0000000000..125848cc47 --- /dev/null +++ b/disperser/dataapi/nonsigner_handlers.go @@ -0,0 +1,163 @@ +package dataapi + +import "fmt" + +type OperatorQuorum struct { + Operator string + QuorumNumbers []byte + BlockNumber uint32 +} + +// Representing an interval [StartBlock, EndBlock] (inclusive). +type BlockInterval struct { + StartBlock uint32 + EndBlock uint32 +} + +// OperatorQuorumIntervals[op][q] is a sequence of increasing and non-overlapping +// intervals during which the operator "op" is registered in quorum "q". +type OperatorQuorumIntervals map[string]map[uint8][]BlockInterval + +// GetQuorums returns the quorums the operator is registered in at the given block number. +func (oqi OperatorQuorumIntervals) GetQuorums(operatorId string, blockNum uint32) []uint8 { + quorums := make([]uint8, 0) + for q, intervals := range oqi[operatorId] { + // Note: if len(intervals) is large, we can perform binary search here. + // In practice it shouldn't be quite small given that the quorum change is + // not frequent, so search it with brute force here. + live := false + for _, interval := range intervals { + if interval.StartBlock > blockNum { + break + } + if blockNum <= interval.EndBlock { + live = true + break + } + } + if live { + quorums = append(quorums, q) + } + } + return quorums +} + +// CreateOperatorQuorumIntervals creates OperatorQuorumIntervals that are within the +// the block interval [startBlock, endBlock] for operators. +// +// The initial quorums at startBlock for all operators are provided by +// "operatorInitialQuorum", and the quorum change events during [startBlock + 1, endBlock] +// are provided by "addedToQuorum" and "removedFromQuorum". +func CreateOperatorQuorumIntervals( + startBlock uint32, + endBlock uint32, + operatorInitialQuorum map[string][]uint8, + addedToQuorum map[string][]*OperatorQuorum, + removedFromQuorum map[string][]*OperatorQuorum, +) (OperatorQuorumIntervals, error) { + if startBlock > endBlock { + return nil, fmt.Errorf("the startBlock must be no less than endBlock, but found startBlock: %d, endBlock: %d", startBlock, endBlock) + } + operatorQuorumIntervals := make(OperatorQuorumIntervals) + for op, initialQuorums := range operatorInitialQuorum { + if len(initialQuorums) == 0 { + return nil, fmt.Errorf("the operator: %s must be in at least one quorum at block: %d", op, startBlock) + } + operatorQuorumIntervals[op] = make(map[uint8][]BlockInterval) + openQuorum := make(map[uint8]uint32) + for _, q := range initialQuorums { + openQuorum[q] = startBlock + } + added := addedToQuorum[op] + removed := removedFromQuorum[op] + i, j := 0, 0 + for i < len(added) && j < len(removed) { + // Skip the block if it's not after startBlock, because the operatorInitialQuorum + // already gives the state at startBlock. + if added[i].BlockNumber <= startBlock { + i++ + continue + } + if removed[j].BlockNumber <= startBlock { + j++ + continue + } + // TODO: Having quorum addition and removal in the same block is a valid case. + // Will come up a followup fix to handle this special case. + if added[i].BlockNumber == removed[j].BlockNumber { + return nil, fmt.Errorf("Not yet supported: the operator: %s was adding and removing quorums at the same block number: %d", op, added[i].BlockNumber) + } + if added[i].BlockNumber < removed[j].BlockNumber { + for _, q := range added[i].QuorumNumbers { + start, ok := openQuorum[q] + if ok { + return nil, fmt.Errorf("cannot add operator: %s to quorum: %d at block number: %d, because it is already in the quorum since block number: %d", op, q, start, added[i].BlockNumber) + } + openQuorum[q] = added[i].BlockNumber + } + i++ + } else { + err := removeQuorums(removed[j], openQuorum, operatorQuorumIntervals) + if err != nil { + return nil, err + } + j++ + } + } + for ; i < len(added); i++ { + for _, q := range added[i].QuorumNumbers { + start, ok := openQuorum[q] + if ok { + return nil, fmt.Errorf("cannot add operator: %s to quorum: %d at block number: %d, because it is already in the quorum since block number: %d", op, q, start, added[i].BlockNumber) + } + openQuorum[q] = added[i].BlockNumber + } + } + for ; j < len(removed); j++ { + err := removeQuorums(removed[j], openQuorum, operatorQuorumIntervals) + if err != nil { + return nil, err + } + } + for q, start := range openQuorum { + interval := BlockInterval{ + StartBlock: start, + EndBlock: endBlock, + } + _, ok := operatorQuorumIntervals[op][q] + if !ok { + operatorQuorumIntervals[op][q] = make([]BlockInterval, 0) + } + operatorQuorumIntervals[op][q] = append(operatorQuorumIntervals[op][q], interval) + } + } + + return operatorQuorumIntervals, nil +} + +// removeQuorums handles a quorum removal event, which marks the end of membership in a +// quorum, so it'll form a block interval. +func removeQuorums(operatorQuorum *OperatorQuorum, openQuorum map[uint8]uint32, result OperatorQuorumIntervals) error { + op := operatorQuorum.Operator + for _, q := range operatorQuorum.QuorumNumbers { + start, ok := openQuorum[q] + if !ok { + return fmt.Errorf("cannot remove a quorum: %d, because the operator: %s is not in the quorum at block number: %d", q, op, operatorQuorum.BlockNumber) + } + if start >= operatorQuorum.BlockNumber { + return fmt.Errorf("deregistration block number: %d must be strictly greater than its registration block number: %d, for operator: %s, quorum: %d", operatorQuorum.BlockNumber, start, op, q) + } + interval := BlockInterval{ + StartBlock: start, + // The operator is NOT live at the block it's deregistered. + EndBlock: operatorQuorum.BlockNumber - 1, + } + _, ok = result[op][q] + if !ok { + result[op][q] = make([]BlockInterval, 0) + } + result[op][q] = append(result[op][q], interval) + delete(openQuorum, q) + } + return nil +} diff --git a/disperser/dataapi/nonsigner_handlers_test.go b/disperser/dataapi/nonsigner_handlers_test.go new file mode 100644 index 0000000000..1d26c60d87 --- /dev/null +++ b/disperser/dataapi/nonsigner_handlers_test.go @@ -0,0 +1,372 @@ +package dataapi_test + +import ( + "reflect" + "strings" + "testing" + + "github.com/Layr-Labs/eigenda/disperser/dataapi" + "github.com/stretchr/testify/assert" +) + +func assertEntry(t *testing.T, quorumIntervals dataapi.OperatorQuorumIntervals, operator string, expected map[uint8][]dataapi.BlockInterval) { + op, ok := quorumIntervals[operator] + assert.True(t, ok) + assert.True(t, reflect.DeepEqual(op, expected)) +} + +func TestCreateOperatorQuorumIntervalsWithInvalidArgs(t *testing.T) { + addedQuorums := map[string][]*dataapi.OperatorQuorum{} + removedQuorums := map[string][]*dataapi.OperatorQuorum{} + + // Empty initial quorums + operatorInitialQuorum := map[string][]uint8{ + "operator-1": {}, + "operator-2": {0x01}, + } + _, err := dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "must be in at least one quorum")) + + // StartBlock > EndBlock + operatorInitialQuorum = map[string][]uint8{ + "operator-1": {0x00}, + "operator-2": {0x00}, + } + _, err = dataapi.CreateOperatorQuorumIntervals(100, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "startBlock must be no less than endBlock")) + + // Equal block number + addedQuorums = map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x01}, + BlockNumber: 12, + }, + }, + } + removedQuorums = map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x00}, + BlockNumber: 12, + }, + }, + } + _, err = dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "adding and removing quorums at the same block")) + + // Adding existing quorum again + addedQuorums = map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x00}, + BlockNumber: 11, + }, + }, + } + _, err = dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "it is already in the quorum")) + + // Removing nonexisting quorum + addedQuorums = map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x02}, + BlockNumber: 12, + }, + }, + } + removedQuorums = map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x01}, + BlockNumber: 11, + }, + }, + } + _, err = dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "cannot remove a quorum")) +} + +func TestCreateOperatorQuorumIntervalsWithNoQuorumChanges(t *testing.T) { + addedQuorums := map[string][]*dataapi.OperatorQuorum{} + removedQuorums := map[string][]*dataapi.OperatorQuorum{} + operatorInitialQuorum := map[string][]uint8{ + "operator-1": {0x00}, + "operator-2": {0x01}, + } + quorumIntervals, err := dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.NoError(t, err) + + assert.Equal(t, 2, len(quorumIntervals)) + expectedOp1 := map[uint8][]dataapi.BlockInterval{0: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 25, + }, + }, + } + assertEntry(t, quorumIntervals, "operator-1", expectedOp1) + expectedOp2 := map[uint8][]dataapi.BlockInterval{ + 1: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 25, + }, + }, + } + assertEntry(t, quorumIntervals, "operator-2", expectedOp2) +} + +func TestCreateOperatorQuorumIntervalsWithOnlyAddOrRemove(t *testing.T) { + addedQuorums := map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x01}, + BlockNumber: 11, + }, + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x02, 0x03}, + BlockNumber: 20, + }, + }, + "operator-2": []*dataapi.OperatorQuorum{ + { + Operator: "operator-2", + QuorumNumbers: []byte{0x01, 0x02}, + BlockNumber: 25, + }, + }, + } + removedQuorums := map[string][]*dataapi.OperatorQuorum{} + operatorInitialQuorum := map[string][]uint8{ + "operator-1": {0x00}, + "operator-2": {0x00}, + } + + quorumIntervals, err := dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.NoError(t, err) + + assert.Equal(t, 2, len(quorumIntervals)) + expectedOp1 := map[uint8][]dataapi.BlockInterval{ + 0: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 25, + }, + }, + 1: []dataapi.BlockInterval{ + { + StartBlock: 11, + EndBlock: 25, + }, + }, + 2: []dataapi.BlockInterval{ + { + StartBlock: 20, + EndBlock: 25, + }, + }, + 3: []dataapi.BlockInterval{ + { + StartBlock: 20, + EndBlock: 25, + }, + }, + } + assertEntry(t, quorumIntervals, "operator-1", expectedOp1) + + expectedOp2 := map[uint8][]dataapi.BlockInterval{ + 0: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 25, + }, + }, + 1: []dataapi.BlockInterval{ + { + StartBlock: 25, + EndBlock: 25, + }, + }, + 2: []dataapi.BlockInterval{ + { + StartBlock: 25, + EndBlock: 25, + }, + }, + } + assertEntry(t, quorumIntervals, "operator-2", expectedOp2) + + addedQuorums = map[string][]*dataapi.OperatorQuorum{} + removedQuorums = map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x00}, + BlockNumber: 15, + }, + }, + } + quorumIntervals, err = dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.NoError(t, err) + expectedOp3 := map[uint8][]dataapi.BlockInterval{ + 0: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 14, + }, + }, + } + assertEntry(t, quorumIntervals, "operator-1", expectedOp3) +} + +func TestCreateOperatorQuorumIntervals(t *testing.T) { + addedQuorums := map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x01}, + BlockNumber: 11, + }, + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x02, 0x03}, + BlockNumber: 20, + }, + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x00}, + BlockNumber: 20, + }, + }, + "operator-2": []*dataapi.OperatorQuorum{ + { + Operator: "operator-2", + QuorumNumbers: []byte{0x02}, + BlockNumber: 15, + }, + { + Operator: "operator-2", + QuorumNumbers: []byte{0x02}, + BlockNumber: 22, + }, + }, + } + removedQuorums := map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x00}, + BlockNumber: 15, + }, + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x02}, + BlockNumber: 21, + }, + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x00}, + BlockNumber: 23, + }, + }, + "operator-2": []*dataapi.OperatorQuorum{ + { + Operator: "operator-2", + QuorumNumbers: []byte{0x01, 0x02}, + BlockNumber: 20, + }, + }, + } + operatorInitialQuorum := map[string][]uint8{ + "operator-1": {0x00}, + "operator-2": {0x00, 0x01}, + } + + quorumIntervals, err := dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.NoError(t, err) + + assert.Equal(t, 2, len(quorumIntervals)) + expectedOp1 := map[uint8][]dataapi.BlockInterval{ + 0: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 14, + }, + { + StartBlock: 20, + EndBlock: 22, + }, + }, + 1: []dataapi.BlockInterval{ + { + StartBlock: 11, + EndBlock: 25, + }, + }, + 2: []dataapi.BlockInterval{ + { + StartBlock: 20, + EndBlock: 20, + }, + }, + 3: []dataapi.BlockInterval{ + { + StartBlock: 20, + EndBlock: 25, + }, + }, + } + assertEntry(t, quorumIntervals, "operator-1", expectedOp1) + assert.ElementsMatch(t, []uint8{0x00}, quorumIntervals.GetQuorums("operator-1", 10)) + assert.ElementsMatch(t, []uint8{0x00, 0x01}, quorumIntervals.GetQuorums("operator-1", 11)) + assert.ElementsMatch(t, []uint8{0x01}, quorumIntervals.GetQuorums("operator-1", 15)) + assert.ElementsMatch(t, []uint8{0x00, 0x01, 0x02, 0x03}, quorumIntervals.GetQuorums("operator-1", 20)) + assert.ElementsMatch(t, []uint8{0x00, 0x01, 0x03}, quorumIntervals.GetQuorums("operator-1", 22)) + assert.ElementsMatch(t, []uint8{0x01, 0x03}, quorumIntervals.GetQuorums("operator-1", 23)) + assert.ElementsMatch(t, []uint8{0x01, 0x03}, quorumIntervals.GetQuorums("operator-1", 25)) + + expectedOp2 := map[uint8][]dataapi.BlockInterval{ + 0: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 25, + }, + }, + 1: []dataapi.BlockInterval{ + { + StartBlock: 10, + EndBlock: 19, + }, + }, + 2: []dataapi.BlockInterval{ + { + StartBlock: 15, + EndBlock: 19, + }, + { + StartBlock: 22, + EndBlock: 25, + }, + }, + } + assertEntry(t, quorumIntervals, "operator-2", expectedOp2) + assert.ElementsMatch(t, []uint8{0x00, 0x01}, quorumIntervals.GetQuorums("operator-2", 10)) + assert.ElementsMatch(t, []uint8{0x00, 0x01, 0x02}, quorumIntervals.GetQuorums("operator-2", 15)) + assert.ElementsMatch(t, []uint8{0x00}, quorumIntervals.GetQuorums("operator-2", 20)) + assert.ElementsMatch(t, []uint8{0x00, 0x02}, quorumIntervals.GetQuorums("operator-2", 22)) + assert.ElementsMatch(t, []uint8{0x00, 0x02}, quorumIntervals.GetQuorums("operator-2", 25)) +} From b41e1d8206b5be2337dba2e79038adcab4a67d53 Mon Sep 17 00:00:00 2001 From: Jian Xiao Date: Sat, 2 Mar 2024 22:18:23 +0000 Subject: [PATCH 2/7] fix --- disperser/dataapi/nonsigner_handlers.go | 79 ++++++++++++++------ disperser/dataapi/nonsigner_handlers_test.go | 22 +++++- 2 files changed, 78 insertions(+), 23 deletions(-) diff --git a/disperser/dataapi/nonsigner_handlers.go b/disperser/dataapi/nonsigner_handlers.go index 125848cc47..59099e31f7 100644 --- a/disperser/dataapi/nonsigner_handlers.go +++ b/disperser/dataapi/nonsigner_handlers.go @@ -23,7 +23,7 @@ func (oqi OperatorQuorumIntervals) GetQuorums(operatorId string, blockNum uint32 quorums := make([]uint8, 0) for q, intervals := range oqi[operatorId] { // Note: if len(intervals) is large, we can perform binary search here. - // In practice it shouldn't be quite small given that the quorum change is + // In practice it should be quite small given that the quorum change is // not frequent, so search it with brute force here. live := false for _, interval := range intervals { @@ -45,9 +45,17 @@ func (oqi OperatorQuorumIntervals) GetQuorums(operatorId string, blockNum uint32 // CreateOperatorQuorumIntervals creates OperatorQuorumIntervals that are within the // the block interval [startBlock, endBlock] for operators. // -// The initial quorums at startBlock for all operators are provided by -// "operatorInitialQuorum", and the quorum change events during [startBlock + 1, endBlock] -// are provided by "addedToQuorum" and "removedFromQuorum". +// The parameters: +// - startBlock, endBlock: specifying the block interval that's of interest. +// Requires: startBlock <= endBlock. +// - operatorInitialQuorum: the initial quorums at startBlock that operators were +// registered in. +// Requires: operatorInitialQuorum[op] is non-empty for each operator "op". +// - addedToQuorum, removedFromQuorum: a sequence of events that added/removed operators +// to/from quorums. +// Requires: +// 1) the block number for all events are in range [startBlock+1, endBlock]; +// 2) the events are in ascending order by block number for each operator "op". func CreateOperatorQuorumIntervals( startBlock uint32, endBlock uint32, @@ -56,12 +64,14 @@ func CreateOperatorQuorumIntervals( removedFromQuorum map[string][]*OperatorQuorum, ) (OperatorQuorumIntervals, error) { if startBlock > endBlock { - return nil, fmt.Errorf("the startBlock must be no less than endBlock, but found startBlock: %d, endBlock: %d", startBlock, endBlock) + msg := "the startBlock must be no less than endBlock, but found " + + "startBlock: %d, endBlock: %d" + return nil, fmt.Errorf(msg, startBlock, endBlock) } operatorQuorumIntervals := make(OperatorQuorumIntervals) for op, initialQuorums := range operatorInitialQuorum { if len(initialQuorums) == 0 { - return nil, fmt.Errorf("the operator: %s must be in at least one quorum at block: %d", op, startBlock) + return nil, fmt.Errorf("operator %s must be in at least one quorum at block %d", op, startBlock) } operatorQuorumIntervals[op] = make(map[uint8][]BlockInterval) openQuorum := make(map[uint8]uint32) @@ -70,28 +80,26 @@ func CreateOperatorQuorumIntervals( } added := addedToQuorum[op] removed := removedFromQuorum[op] + eventErr := validateQuorumEvents(added, removed, startBlock, endBlock) + if eventErr != nil { + return nil, eventErr + } i, j := 0, 0 for i < len(added) && j < len(removed) { - // Skip the block if it's not after startBlock, because the operatorInitialQuorum - // already gives the state at startBlock. - if added[i].BlockNumber <= startBlock { - i++ - continue - } - if removed[j].BlockNumber <= startBlock { - j++ - continue - } - // TODO: Having quorum addition and removal in the same block is a valid case. + // TODO(jianoaix): Having quorum addition and removal in the same block is a valid case. // Will come up a followup fix to handle this special case. if added[i].BlockNumber == removed[j].BlockNumber { - return nil, fmt.Errorf("Not yet supported: the operator: %s was adding and removing quorums at the same block number: %d", op, added[i].BlockNumber) + msg := "Not yet supported: operator was adding and removing quorums at the " + + "same block. operator: %s, block number: %d" + return nil, fmt.Errorf(msg, op, added[i].BlockNumber) } if added[i].BlockNumber < removed[j].BlockNumber { for _, q := range added[i].QuorumNumbers { start, ok := openQuorum[q] if ok { - return nil, fmt.Errorf("cannot add operator: %s to quorum: %d at block number: %d, because it is already in the quorum since block number: %d", op, q, start, added[i].BlockNumber) + msg := "cannot add operator %s to quorum %d at block number %d, " + + "the operator is already in the quorum since block number: %d" + return nil, fmt.Errorf(msg, op, q, added[i].BlockNumber, start) } openQuorum[q] = added[i].BlockNumber } @@ -108,7 +116,9 @@ func CreateOperatorQuorumIntervals( for _, q := range added[i].QuorumNumbers { start, ok := openQuorum[q] if ok { - return nil, fmt.Errorf("cannot add operator: %s to quorum: %d at block number: %d, because it is already in the quorum since block number: %d", op, q, start, added[i].BlockNumber) + msg := "cannot add operator %s to quorum %d at block number %d, " + + "the operator is already in the quorum since block number: %d" + return nil, fmt.Errorf(msg, op, q, added[i].BlockNumber, start) } openQuorum[q] = added[i].BlockNumber } @@ -142,10 +152,14 @@ func removeQuorums(operatorQuorum *OperatorQuorum, openQuorum map[uint8]uint32, for _, q := range operatorQuorum.QuorumNumbers { start, ok := openQuorum[q] if !ok { - return fmt.Errorf("cannot remove a quorum: %d, because the operator: %s is not in the quorum at block number: %d", q, op, operatorQuorum.BlockNumber) + msg := "cannot remove a quorum %d, the operator %s is not yet in the quorum " + + "at block %d" + return fmt.Errorf(msg, q, op, operatorQuorum.BlockNumber) } if start >= operatorQuorum.BlockNumber { - return fmt.Errorf("deregistration block number: %d must be strictly greater than its registration block number: %d, for operator: %s, quorum: %d", operatorQuorum.BlockNumber, start, op, q) + msg := "deregistration block number %d must be strictly greater than its " + + "registration block number %d, for operator %s, quorum %d" + return fmt.Errorf(msg, operatorQuorum.BlockNumber, start, op, q) } interval := BlockInterval{ StartBlock: start, @@ -161,3 +175,24 @@ func removeQuorums(operatorQuorum *OperatorQuorum, openQuorum map[uint8]uint32, } return nil } + +// validateQuorumEvents validates the operator quorum events have the desired block numbers and +// are in ascending order by block number. +func validateQuorumEvents(added []*OperatorQuorum, removed []*OperatorQuorum, startBlock, endBlock uint32) error { + validate := func(events []*OperatorQuorum) error { + for i := range events { + if events[i].BlockNumber <= startBlock || events[i].BlockNumber > endBlock { + return fmt.Errorf("quorum events must be in range [%d, %d]", startBlock+1, endBlock) + } + if i > 0 && events[i].BlockNumber < events[i-1].BlockNumber { + return fmt.Errorf("quorum events must be in ascending order by block number") + } + } + return nil + } + err := validate(added) + if err != nil { + return err + } + return validate(removed) +} diff --git a/disperser/dataapi/nonsigner_handlers_test.go b/disperser/dataapi/nonsigner_handlers_test.go index 1d26c60d87..f093b6bdf6 100644 --- a/disperser/dataapi/nonsigner_handlers_test.go +++ b/disperser/dataapi/nonsigner_handlers_test.go @@ -72,7 +72,27 @@ func TestCreateOperatorQuorumIntervalsWithInvalidArgs(t *testing.T) { } _, err = dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) assert.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "it is already in the quorum")) + assert.True(t, strings.Contains(err.Error(), "operator is already in the quorum")) + + // addedQuurums not in ascending order of block number + addedQuorums = map[string][]*dataapi.OperatorQuorum{ + "operator-1": []*dataapi.OperatorQuorum{ + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x01}, + BlockNumber: 15, + }, + { + Operator: "operator-1", + QuorumNumbers: []uint8{0x03}, + BlockNumber: 11, + }, + }, + } + _, err = dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) + assert.Error(t, err) + // fmt.Println("XX: ", err.Error()) + assert.True(t, strings.Contains(err.Error(), "must be in ascending order by block number")) // Removing nonexisting quorum addedQuorums = map[string][]*dataapi.OperatorQuorum{ From 2dbf49aee82ceb4ebd3e78076d6dfb69dd831671 Mon Sep 17 00:00:00 2001 From: Jian Xiao Date: Sat, 2 Mar 2024 22:24:58 +0000 Subject: [PATCH 3/7] fix --- disperser/dataapi/nonsigner_handlers.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/disperser/dataapi/nonsigner_handlers.go b/disperser/dataapi/nonsigner_handlers.go index 59099e31f7..60355f093d 100644 --- a/disperser/dataapi/nonsigner_handlers.go +++ b/disperser/dataapi/nonsigner_handlers.go @@ -54,7 +54,7 @@ func (oqi OperatorQuorumIntervals) GetQuorums(operatorId string, blockNum uint32 // - addedToQuorum, removedFromQuorum: a sequence of events that added/removed operators // to/from quorums. // Requires: -// 1) the block number for all events are in range [startBlock+1, endBlock]; +// 1) the block numbers for all events are in range [startBlock+1, endBlock]; // 2) the events are in ascending order by block number for each operator "op". func CreateOperatorQuorumIntervals( startBlock uint32, @@ -69,6 +69,8 @@ func CreateOperatorQuorumIntervals( return nil, fmt.Errorf(msg, startBlock, endBlock) } operatorQuorumIntervals := make(OperatorQuorumIntervals) + addedToQuorumErr := "cannot add operator %s to quorum %d at block number %d, " + + "the operator is already in the quorum since block number: %d" for op, initialQuorums := range operatorInitialQuorum { if len(initialQuorums) == 0 { return nil, fmt.Errorf("operator %s must be in at least one quorum at block %d", op, startBlock) @@ -87,7 +89,7 @@ func CreateOperatorQuorumIntervals( i, j := 0, 0 for i < len(added) && j < len(removed) { // TODO(jianoaix): Having quorum addition and removal in the same block is a valid case. - // Will come up a followup fix to handle this special case. + // Come up a followup fix to handle this special case. if added[i].BlockNumber == removed[j].BlockNumber { msg := "Not yet supported: operator was adding and removing quorums at the " + "same block. operator: %s, block number: %d" @@ -97,9 +99,7 @@ func CreateOperatorQuorumIntervals( for _, q := range added[i].QuorumNumbers { start, ok := openQuorum[q] if ok { - msg := "cannot add operator %s to quorum %d at block number %d, " + - "the operator is already in the quorum since block number: %d" - return nil, fmt.Errorf(msg, op, q, added[i].BlockNumber, start) + return nil, fmt.Errorf(addedToQuorumErr, op, q, added[i].BlockNumber, start) } openQuorum[q] = added[i].BlockNumber } @@ -116,9 +116,7 @@ func CreateOperatorQuorumIntervals( for _, q := range added[i].QuorumNumbers { start, ok := openQuorum[q] if ok { - msg := "cannot add operator %s to quorum %d at block number %d, " + - "the operator is already in the quorum since block number: %d" - return nil, fmt.Errorf(msg, op, q, added[i].BlockNumber, start) + return nil, fmt.Errorf(addedToQuorumErr, op, q, added[i].BlockNumber, start) } openQuorum[q] = added[i].BlockNumber } From 966596a2cd59687b7747d2d8a236b04862341bda Mon Sep 17 00:00:00 2001 From: Jian Xiao Date: Sat, 2 Mar 2024 22:26:31 +0000 Subject: [PATCH 4/7] fix --- disperser/dataapi/{nonsigner_handlers.go => nonsigner_utils.go} | 0 .../{nonsigner_handlers_test.go => nonsigner_utils_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename disperser/dataapi/{nonsigner_handlers.go => nonsigner_utils.go} (100%) rename disperser/dataapi/{nonsigner_handlers_test.go => nonsigner_utils_test.go} (100%) diff --git a/disperser/dataapi/nonsigner_handlers.go b/disperser/dataapi/nonsigner_utils.go similarity index 100% rename from disperser/dataapi/nonsigner_handlers.go rename to disperser/dataapi/nonsigner_utils.go diff --git a/disperser/dataapi/nonsigner_handlers_test.go b/disperser/dataapi/nonsigner_utils_test.go similarity index 100% rename from disperser/dataapi/nonsigner_handlers_test.go rename to disperser/dataapi/nonsigner_utils_test.go From d558abaaec9148805c9b47e80471c3ff8013f5f4 Mon Sep 17 00:00:00 2001 From: Jian Xiao Date: Sat, 2 Mar 2024 22:31:43 +0000 Subject: [PATCH 5/7] fix --- disperser/dataapi/nonsigner_utils_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/disperser/dataapi/nonsigner_utils_test.go b/disperser/dataapi/nonsigner_utils_test.go index f093b6bdf6..e2ab7d6a59 100644 --- a/disperser/dataapi/nonsigner_utils_test.go +++ b/disperser/dataapi/nonsigner_utils_test.go @@ -91,7 +91,6 @@ func TestCreateOperatorQuorumIntervalsWithInvalidArgs(t *testing.T) { } _, err = dataapi.CreateOperatorQuorumIntervals(10, 25, operatorInitialQuorum, addedQuorums, removedQuorums) assert.Error(t, err) - // fmt.Println("XX: ", err.Error()) assert.True(t, strings.Contains(err.Error(), "must be in ascending order by block number")) // Removing nonexisting quorum From 6b29570ec1f79dac90ae09de9c65b9e62a8682e7 Mon Sep 17 00:00:00 2001 From: Jian Xiao Date: Sun, 3 Mar 2024 03:37:25 +0000 Subject: [PATCH 6/7] fix --- disperser/dataapi/nonsigner_utils.go | 42 +++++++++-------------- disperser/dataapi/nonsigner_utils_test.go | 2 +- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/disperser/dataapi/nonsigner_utils.go b/disperser/dataapi/nonsigner_utils.go index 60355f093d..21bf51b217 100644 --- a/disperser/dataapi/nonsigner_utils.go +++ b/disperser/dataapi/nonsigner_utils.go @@ -46,7 +46,7 @@ func (oqi OperatorQuorumIntervals) GetQuorums(operatorId string, blockNum uint32 // the block interval [startBlock, endBlock] for operators. // // The parameters: -// - startBlock, endBlock: specifying the block interval that's of interest. +// - startBlock, endBlock: specifying the block interval of interest. // Requires: startBlock <= endBlock. // - operatorInitialQuorum: the initial quorums at startBlock that operators were // registered in. @@ -64,13 +64,13 @@ func CreateOperatorQuorumIntervals( removedFromQuorum map[string][]*OperatorQuorum, ) (OperatorQuorumIntervals, error) { if startBlock > endBlock { - msg := "the startBlock must be no less than endBlock, but found " + + msg := "the endBlock must be no less than startBlock, but found " + "startBlock: %d, endBlock: %d" return nil, fmt.Errorf(msg, startBlock, endBlock) } operatorQuorumIntervals := make(OperatorQuorumIntervals) addedToQuorumErr := "cannot add operator %s to quorum %d at block number %d, " + - "the operator is already in the quorum since block number: %d" + "the operator is already in the quorum since block number %d" for op, initialQuorums := range operatorInitialQuorum { if len(initialQuorums) == 0 { return nil, fmt.Errorf("operator %s must be in at least one quorum at block %d", op, startBlock) @@ -82,8 +82,7 @@ func CreateOperatorQuorumIntervals( } added := addedToQuorum[op] removed := removedFromQuorum[op] - eventErr := validateQuorumEvents(added, removed, startBlock, endBlock) - if eventErr != nil { + if eventErr := validateQuorumEvents(added, removed, startBlock, endBlock); eventErr != nil { return nil, eventErr } i, j := 0, 0 @@ -92,21 +91,19 @@ func CreateOperatorQuorumIntervals( // Come up a followup fix to handle this special case. if added[i].BlockNumber == removed[j].BlockNumber { msg := "Not yet supported: operator was adding and removing quorums at the " + - "same block. operator: %s, block number: %d" + "same block, operator: %s, block number: %d" return nil, fmt.Errorf(msg, op, added[i].BlockNumber) } if added[i].BlockNumber < removed[j].BlockNumber { for _, q := range added[i].QuorumNumbers { - start, ok := openQuorum[q] - if ok { + if start, ok := openQuorum[q]; ok { return nil, fmt.Errorf(addedToQuorumErr, op, q, added[i].BlockNumber, start) } openQuorum[q] = added[i].BlockNumber } i++ } else { - err := removeQuorums(removed[j], openQuorum, operatorQuorumIntervals) - if err != nil { + if err := removeQuorums(removed[j], openQuorum, operatorQuorumIntervals); err != nil { return nil, err } j++ @@ -114,16 +111,14 @@ func CreateOperatorQuorumIntervals( } for ; i < len(added); i++ { for _, q := range added[i].QuorumNumbers { - start, ok := openQuorum[q] - if ok { + if start, ok := openQuorum[q]; ok { return nil, fmt.Errorf(addedToQuorumErr, op, q, added[i].BlockNumber, start) } openQuorum[q] = added[i].BlockNumber } } for ; j < len(removed); j++ { - err := removeQuorums(removed[j], openQuorum, operatorQuorumIntervals) - if err != nil { + if err := removeQuorums(removed[j], openQuorum, operatorQuorumIntervals); err != nil { return nil, err } } @@ -132,8 +127,7 @@ func CreateOperatorQuorumIntervals( StartBlock: start, EndBlock: endBlock, } - _, ok := operatorQuorumIntervals[op][q] - if !ok { + if _, ok := operatorQuorumIntervals[op][q]; !ok { operatorQuorumIntervals[op][q] = make([]BlockInterval, 0) } operatorQuorumIntervals[op][q] = append(operatorQuorumIntervals[op][q], interval) @@ -143,15 +137,15 @@ func CreateOperatorQuorumIntervals( return operatorQuorumIntervals, nil } -// removeQuorums handles a quorum removal event, which marks the end of membership in a -// quorum, so it'll form a block interval. +// removeQuorums handles a quorum removal event, which marks the end of membership in a quorum, +// so it'll form a block interval. func removeQuorums(operatorQuorum *OperatorQuorum, openQuorum map[uint8]uint32, result OperatorQuorumIntervals) error { op := operatorQuorum.Operator for _, q := range operatorQuorum.QuorumNumbers { start, ok := openQuorum[q] if !ok { msg := "cannot remove a quorum %d, the operator %s is not yet in the quorum " + - "at block %d" + "at block number %d" return fmt.Errorf(msg, q, op, operatorQuorum.BlockNumber) } if start >= operatorQuorum.BlockNumber { @@ -164,8 +158,7 @@ func removeQuorums(operatorQuorum *OperatorQuorum, openQuorum map[uint8]uint32, // The operator is NOT live at the block it's deregistered. EndBlock: operatorQuorum.BlockNumber - 1, } - _, ok = result[op][q] - if !ok { + if _, ok = result[op][q]; !ok { result[op][q] = make([]BlockInterval, 0) } result[op][q] = append(result[op][q], interval) @@ -174,8 +167,8 @@ func removeQuorums(operatorQuorum *OperatorQuorum, openQuorum map[uint8]uint32, return nil } -// validateQuorumEvents validates the operator quorum events have the desired block numbers and -// are in ascending order by block number. +// validateQuorumEvents validates the operator quorum events have the desired block numbers and are +// in ascending order by block numbers. func validateQuorumEvents(added []*OperatorQuorum, removed []*OperatorQuorum, startBlock, endBlock uint32) error { validate := func(events []*OperatorQuorum) error { for i := range events { @@ -188,8 +181,7 @@ func validateQuorumEvents(added []*OperatorQuorum, removed []*OperatorQuorum, st } return nil } - err := validate(added) - if err != nil { + if err := validate(added); err != nil { return err } return validate(removed) diff --git a/disperser/dataapi/nonsigner_utils_test.go b/disperser/dataapi/nonsigner_utils_test.go index e2ab7d6a59..7182cec5a1 100644 --- a/disperser/dataapi/nonsigner_utils_test.go +++ b/disperser/dataapi/nonsigner_utils_test.go @@ -35,7 +35,7 @@ func TestCreateOperatorQuorumIntervalsWithInvalidArgs(t *testing.T) { } _, err = dataapi.CreateOperatorQuorumIntervals(100, 25, operatorInitialQuorum, addedQuorums, removedQuorums) assert.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "startBlock must be no less than endBlock")) + assert.True(t, strings.Contains(err.Error(), "endBlock must be no less than startBlock")) // Equal block number addedQuorums = map[string][]*dataapi.OperatorQuorum{ From 39e365cff5533d676005da9aee07f6d9fac787ee Mon Sep 17 00:00:00 2001 From: Jian Xiao Date: Tue, 5 Mar 2024 23:46:22 +0000 Subject: [PATCH 7/7] OperatorQuorum no longer needed --- disperser/dataapi/nonsigner_utils.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/disperser/dataapi/nonsigner_utils.go b/disperser/dataapi/nonsigner_utils.go index 21bf51b217..0401489a96 100644 --- a/disperser/dataapi/nonsigner_utils.go +++ b/disperser/dataapi/nonsigner_utils.go @@ -2,12 +2,6 @@ package dataapi import "fmt" -type OperatorQuorum struct { - Operator string - QuorumNumbers []byte - BlockNumber uint32 -} - // Representing an interval [StartBlock, EndBlock] (inclusive). type BlockInterval struct { StartBlock uint32