Skip to content
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

[node] Add more info on error logs #187

Merged
merged 1 commit into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clients/tests/retrieval_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func setup(t *testing.T) {
}

func mustMakeOpertatorPubKeysPair(t *testing.T) *coreindexer.OperatorPubKeys {
operators := make(map[[32]byte]coreindexer.OperatorPubKeysPair, len(operatorState.Operators))
operators := make(map[core.OperatorID]coreindexer.OperatorPubKeysPair, len(operatorState.Operators))
for operatorId := range operatorState.Operators[0] {
keyPair, err := core.GenRandomBlsKeys()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion core/aggregation.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (a *StdSignatureAggregator) AggregateSignatures(ctx context.Context, state
for numReply := 0; numReply < numOperators; numReply++ {
var err error
r := <-messageChan
operatorIDHex := hexutil.Encode(r.Operator[:])
operatorIDHex := r.Operator.Hex()
operatorAddr, ok := a.OperatorAddresses.Get(r.Operator)
if !ok && a.Transactor != nil {
operatorAddr, err = a.Transactor.OperatorIDToAddress(ctx, r.Operator)
Expand Down
11 changes: 8 additions & 3 deletions core/assignment.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package core

import (
"encoding/hex"
"errors"
"fmt"
"math"
Expand Down Expand Up @@ -28,7 +29,11 @@ var (

// Assignment

type OperatorID = [32]byte
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type aliases are convenient, but I think strict types are safer in general (and allow receiver functions)

type OperatorID [32]byte

func (id OperatorID) Hex() string {
return hex.EncodeToString(id[:])
}

type OperatorIndex = uint

Expand Down Expand Up @@ -156,7 +161,7 @@ func (c *StdAssignmentCoordinator) ValidateChunkLength(state *OperatorState, blo

// Check that the chunk length meets the minimum requirement
if info.ChunkLength < MinChunkLength {
return false, ErrChunkLengthTooSmall
return false, fmt.Errorf("%w: chunk length: %d, min chunk length: %d", ErrChunkLengthTooSmall, info.ChunkLength, MinChunkLength)
}

// Get minimum stake amont
Expand All @@ -183,7 +188,7 @@ func (c *StdAssignmentCoordinator) ValidateChunkLength(state *OperatorState, blo
maxChunkLength = uint(nextPowerOf2(uint64(maxChunkLength)))

if info.ChunkLength > maxChunkLength {
return false, ErrChunkLengthTooLarge
return false, fmt.Errorf("%w: chunk length: %d, max chunk length: %d", ErrChunkLengthTooLarge, info.ChunkLength, maxChunkLength)
}

}
Expand Down
2 changes: 1 addition & 1 deletion core/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (s *Signature) Verify(pubkey *G2Point, message [32]byte) bool {
func (p *G1Point) GetOperatorID() OperatorID {
x := p.X.BigInt(new(big.Int))
y := p.Y.BigInt(new(big.Int))
return crypto.Keccak256Hash(append(math.U256Bytes(x), math.U256Bytes(y)...))
return OperatorID(crypto.Keccak256Hash(append(math.U256Bytes(x), math.U256Bytes(y)...)))
}

type PrivateKey = fr.Element
Expand Down
2 changes: 1 addition & 1 deletion core/indexer/operator_sockets_filterer.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (f *operatorSocketsFilterer) WatchOperatorSocketUpdate(ctx context.Context,
}

sink := make(chan *blsregcoord.ContractBLSRegistryCoordinatorWithIndicesOperatorSocketUpdate)
operatorID := []core.OperatorID{operatorId}
operatorID := [][32]byte{operatorId}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended given the goal of this pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this is a consequence of making core.OperatorID an explicit type - contract bindings aren't aware of core.OperatorID, so needs to be converted to a primitive type

_, err = filterer.WatchOperatorSocketUpdate(&bind.WatchOpts{Context: ctx}, sink, operatorID)
if err != nil {
return nil, err
Expand Down
1 change: 0 additions & 1 deletion core/mock/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

var (
ErrChunkLengthMismatch = errors.New("chunk length mismatch")
ErrInvalidHeader = errors.New("invalid header")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not being used

)

// MockChunkValidator is a mock implementation of ChunkValidator
Expand Down
4 changes: 2 additions & 2 deletions core/thegraph/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (ics *indexedChainState) GetIndexedOperatorInfoByOperatorId(ctx context.Con
var (
query QueryOperatorByIdGql
variables = map[string]any{
"id": graphql.String(fmt.Sprintf("0x%s", hex.EncodeToString(operatorId[:]))),
"id": graphql.String(fmt.Sprintf("0x%s", operatorId.Hex())),
}
)
err := ics.querier.Query(context.Background(), &query, variables)
Expand Down Expand Up @@ -235,7 +235,7 @@ func (ics *indexedChainState) getRegisteredIndexedOperatorInfo(ctx context.Conte
return nil, err
}

operators := make(map[[32]byte]*core.IndexedOperatorInfo, len(operatorsGql))
operators := make(map[core.OperatorID]*core.IndexedOperatorInfo, len(operatorsGql))
for i := range operatorsGql {
operator := operatorsGql[i]
operatorIndexedInfo, err := convertIndexedOperatorInfoGqlToIndexedOperatorInfo(&operator)
Expand Down
2 changes: 1 addition & 1 deletion core/thegraph/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func TestIndexedChainState_GetIndexedOperatorInfoByOperatorId(t *testing.T) {
assert.NoError(t, err)

opID := ethcomm.HexToHash("0x3eb7d5df61c48ec2718d8c8ad52304effc970ae92f19138e032dae07b7c0d629")
info, err := cs.GetIndexedOperatorInfoByOperatorId(context.Background(), opID, uint32(headerNum))
info, err := cs.GetIndexedOperatorInfoByOperatorId(context.Background(), core.OperatorID(opID.Bytes()), uint32(headerNum))
assert.NoError(t, err)
assert.Equal(t, "3336192159512049190945679273141887248666932624338963482128432381981287252980", info.PubkeyG1.X.String())
assert.Equal(t, "15195175002875833468883745675063986308012687914999552116603423331534089122704", info.PubkeyG1.Y.String())
Expand Down
17 changes: 8 additions & 9 deletions core/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

var (
ErrChunkLengthMismatch = errors.New("chunk length mismatch")
ErrInvalidHeader = errors.New("invalid header")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not being used

ErrBlobQuorumSkip = errors.New("blob skipped for a quorum before verification")
)

Expand Down Expand Up @@ -38,12 +37,12 @@ func NewChunkValidator(enc Encoder, asgn AssignmentCoordinator, cst ChainState,

func (v *chunkValidator) validateBlobQuorum(quorumHeader *BlobQuorumInfo, blob *BlobMessage, operatorState *OperatorState) ([]*Chunk, *Assignment, *EncodingParams, error) {
if quorumHeader.AdversaryThreshold >= quorumHeader.QuorumThreshold {
return nil, nil, nil, errors.New("invalid header: quorum threshold does not exceed adversary threshold")
return nil, nil, nil, fmt.Errorf("invalid header: quorum threshold (%d) does not exceed adversary threshold (%d)", quorumHeader.QuorumThreshold, quorumHeader.AdversaryThreshold)
}

// Check if the operator is a member of the quorum
if _, ok := operatorState.Operators[quorumHeader.QuorumID]; !ok {
return nil, nil, nil, ErrBlobQuorumSkip
return nil, nil, nil, fmt.Errorf("%w: operator %s is not a member of quorum %d", ErrBlobQuorumSkip, v.operatorID.Hex(), quorumHeader.QuorumID)
}

// Get the assignments for the quorum
Expand All @@ -54,10 +53,10 @@ func (v *chunkValidator) validateBlobQuorum(quorumHeader *BlobQuorumInfo, blob *

// Validate the number of chunks
if assignment.NumChunks == 0 {
return nil, nil, nil, ErrBlobQuorumSkip
return nil, nil, nil, fmt.Errorf("%w: operator %s has no chunks in quorum %d", ErrBlobQuorumSkip, v.operatorID.Hex(), quorumHeader.QuorumID)
}
if assignment.NumChunks != uint(len(blob.Bundles[quorumHeader.QuorumID])) {
return nil, nil, nil, errors.New("number of chunks does not match assignment")
return nil, nil, nil, fmt.Errorf("number of chunks (%d) does not match assignment (%d)", len(blob.Bundles[quorumHeader.QuorumID]), assignment.NumChunks)
}

// Validate the chunkLength against the quorum and adversary threshold parameters
Expand All @@ -70,7 +69,7 @@ func (v *chunkValidator) validateBlobQuorum(quorumHeader *BlobQuorumInfo, blob *
chunks := blob.Bundles[quorumHeader.QuorumID]
for _, chunk := range chunks {
if uint(chunk.Length()) != quorumHeader.ChunkLength {
return nil, nil, nil, ErrChunkLengthMismatch
return nil, nil, nil, fmt.Errorf("%w: chunk length (%d) does not match quorum header (%d)", ErrChunkLengthMismatch, chunk.Length(), quorumHeader.ChunkLength)
}
}

Expand All @@ -81,15 +80,15 @@ func (v *chunkValidator) validateBlobQuorum(quorumHeader *BlobQuorumInfo, blob *
}

if params.ChunkLength != quorumHeader.ChunkLength {
return nil, nil, nil, errors.New("invalid chunk length")
return nil, nil, nil, fmt.Errorf("%w: chunk length from encoding parameters (%d) does not match quorum header (%d)", ErrChunkLengthMismatch, params.ChunkLength, quorumHeader.ChunkLength)
}

return chunks, &assignment, &params, nil
}

func (v *chunkValidator) ValidateBlob(blob *BlobMessage, operatorState *OperatorState) error {
if len(blob.Bundles) != len(blob.BlobHeader.QuorumInfos) {
return errors.New("number of bundles does not match number of quorums")
return fmt.Errorf("number of bundles (%d) does not match number of quorums (%d)", len(blob.Bundles), len(blob.BlobHeader.QuorumInfos))
}

// Validate the blob length
Expand Down Expand Up @@ -127,7 +126,7 @@ func (v *chunkValidator) ValidateBatch(blobs []*BlobMessage, operatorState *Oper

for k, blob := range blobs {
if len(blob.Bundles) != len(blob.BlobHeader.QuorumInfos) {
return errors.New("number of bundles does not match number of quorums")
return fmt.Errorf("number of bundles (%d) does not match number of quorums (%d)", len(blob.Bundles), len(blob.BlobHeader.QuorumInfos))
}

// Saved for the blob length validation
Expand Down
Loading