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

[encodingstreamer] Validate quorums #109

Merged

Conversation

ian-shim
Copy link
Contributor

@ian-shim ian-shim commented Dec 6, 2023

Why are these changes needed?

Currently, quorum validation is delegated to the disperser server.
Encoding streamer does not handle invalid quorums, and if there's a request with invalid quorum, the entire iteration fails and encoding loop halts.
This PR addresses this by identifying invalid quorums and filtering out those blobs.

Checks

  • I've made sure the lint is passing in this PR.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@@ -184,7 +184,7 @@ func (c *churner) getOperatorsToChurn(ctx context.Context, quorumIDs []uint8, op
return nil, errors.New("maxOperatorCount is 0")
}

if uint32(len(operatorStakes[i])) < operatorSetParams.MaxOperatorCount {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

possibly a bug in previous implementation.
operatorStakes should be indexed by quorum ID instead of index of quorumIDs

@@ -26,6 +26,8 @@ type OperatorSetParam struct {
ChurnBIPsOfTotalStake uint16
}

type OperatorStakes map[QuorumID]map[OperatorIndex]OperatorStake
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced existing use of [][]OperatorStake with this new type as it's more clear how OperaterStake are indexed

@ian-shim ian-shim marked this pull request as ready for review December 6, 2023 06:20
@ian-shim ian-shim force-pushed the encoding-streamer-validate-quorums branch from 4bd4da3 to e785104 Compare December 6, 2023 17:20
@ian-shim ian-shim force-pushed the encoding-streamer-validate-quorums branch from e785104 to c109d78 Compare January 4, 2024 01:56
Copy link
Collaborator

@mooselumph mooselumph left a comment

Choose a reason for hiding this comment

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

I think their may be a couple of small issues, but looks good overall!

core/eth/state.go Outdated Show resolved Hide resolved
core/eth/state.go Outdated Show resolved Hide resolved
core/eth/tx.go Outdated Show resolved Hide resolved
core/eth/tx.go Outdated Show resolved Hide resolved
core/state.go Show resolved Hide resolved
disperser/batcher/encoding_streamer.go Outdated Show resolved Hide resolved
@ian-shim ian-shim force-pushed the encoding-streamer-validate-quorums branch from c109d78 to 5b597f3 Compare January 5, 2024 23:22
Copy link
Collaborator

@mooselumph mooselumph left a comment

Choose a reason for hiding this comment

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

Looks good

@ian-shim ian-shim merged commit fec9767 into Layr-Labs:master Jan 8, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants