8000 spec: Clearly document the ordering within `VoteInfo` and `ExtendedVoteInfo` · Issue #779 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

spec: Clearly document the ordering within VoteInfo and ExtendedVoteInfo #779

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.

Sign up for GitHub

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

Jump to bottom
Closed
1 of 2 tasks
Tracked by #24
adizere opened this issue May 1, 2023 · 0 comments · Fixed by #2715
Closed
1 of 2 tasks
Tracked by #24

spec: Clearly document the ordering within VoteInfo and ExtendedVoteInfo #779

adizere opened this issue May 1, 2023 · 0 comments · Fixed by #2715
Assignees
@andynog
Labels
abci Application blockchain interface good first issue Good for newcomers spec Specification-related
Milestone
2024-Q2

Comments

@adizere
Copy link
Member
adizere commented May 1, 2023 •
edited by andynog
Loading

Feature Request

Summary

It would be useful if the spec for ABCI 2.0 documented the expected ordering of elements within fields VoteInfo and ExtendedVoteInfo in all the methods where these fields appear. Application developers might expects that these elements are ordered deterministically by validator power; if so, the docs should capture it, and if not, we should also state it.

Problem Definition

Developers who are integrating v0.38 can be confused about the guarantees on the ordering of elements within VoteInfo and ExtendedVoteInfo. There are three ABCI methods concerned here:

Proposal

The method ValidatorSet.Validators already handles ordering:

cometbft/types/validator_set.go

Lines 38 to 53 in 2a0c1e6

// ValidatorSet represent a set of *Validator at a given height.
//
// The validators can be fetched by address or index.
// The index is in order of .VotingPower, so the indices are fixed for all
// rounds of a given blockchain height - ie. the validators are sorted by their
// voting power (descending). Secondary index - .Address (ascending).
//
// On the other hand, the .ProposerPriority of each validator and the
// designated .GetProposer() of a set changes every round, upon calling
// .IncrementProposerPriority().
//
// NOTE: Not goroutine-safe.
// NOTE: All get/set to validators should copy the value for safety.
type ValidatorSet struct {
// NOTE: persisted via reflect, must be exported.
Validators []*Validator `json:"validators"`

This method is used by buildExtendedCommitInfo and buildLastCommitInfo, used in the three ABCI methods described above.

Acceptance criteria:

  • double-check that the above is accurate
  • extend the specification for the three ABCI methods concerned to document the ordering guarantees

H/t Marko B. for noticing this underspecified part of the spec!
@adizere adizere added good first issue Good for newcomers abci Application blockchain interface spec Specification-related labels May 1, 2023
@adizere adizere mentioned this issue May 1, 2023
Closed
11 tasks
@andynog andynog self-assigned this Apr 1, 2024
@andynog andynog added this to CometBFT Apr 1, 2024
@github-project-automation github-project-automation bot moved this to Todo in CometBFT Apr 1, 2024
@andynog andynog moved this from Todo to In Progress in CometBFT Apr 1, 2024
@andynog andynog added this to the 2024-Q2 milestone Apr 1, 2024
andynog added a commit that referenced this issue Apr 3, 2024
@andynog
added information about validator ordering (#779)
6762aed
@andynog andynog mentioned this issue Apr 3, 2024
Merged
4 tasks
@andynog andynog moved this from In Progress to Ready for Review in CometBFT Apr 3, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 4, 2024
@andynog
spec: Add additional information about validator ordering (total powe…
0349b99 
…r) (#2715)

close: #779 

This PR adds additional information on `ExtendedCommitInfo` and
`CommitInfo` data types about the validator set ordering (total power)
guarantees.

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
@github-project-automation github-project-automation bot moved this from Ready for Review to Done in CometBFT Apr 4, 2024
mergify bot pushed a commit that referenced this issue Apr 4, 2024
@andynog @mergify
spec: Add additional information about validator ordering (total powe…
dc99059 
…r) (#2715)

close: #779

This PR adds additional information on `ExtendedCommitInfo` and
`CommitInfo` data types about the validator set ordering (total power)
guarantees.

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 0349b99)
@mergify mergify bot mentioned this issue Apr 4, 2024
Merged
4 tasks
andynog added a commit that referenced this issue Apr 4, 2024
@mergify @andynog
spec: Add additional information about validator ordering (total powe…
4d4ac34 
…r) (backport #2715) (#2720)

close: #779 

This PR adds additional information on `ExtendedCommitInfo` and
`CommitInfo` data types about the validator set ordering (total power)
guarantees.

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2715 done by
[Mergify](https://mergify.com).

Co-authored-by: Andy Nogueira <me@andynogueira.dev>
mergify bot pushed a commit that referenced this issue Apr 5, 2024
@andynog @mergify
spec: Add additional information about validator ordering (total powe…
e34e967 
…r) (#2715)

close: #779

This PR adds additional information on `ExtendedCommitInfo` and
`CommitInfo` data types about the validator set ordering (total power)
guarantees.

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 0349b99)
@mergify mergify bot mentioned this issue Apr 5, 2024
Merged
4 tasks
andynog added a commit that referenced this issue Apr 5, 2024
@mergify @andynog
spec: Add additional information about validator ordering (total powe…
fd5b3f3 
…r) (backport #2715) (#2724)

close: #779 

This PR adds additional information on `ExtendedCommitInfo` and
`CommitInfo` data types about the validator set ordering (total power)
guarantees.

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2715 done by
[Mergify](https://mergify.com).

Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andynog andynog

Labels
abci Application blockchain interface good first issue Good for newcomers spec Specification-related
Projects
No open projects
CometBFT
Status: Done
Milestone
2024-Q2
Development

Successfully merging a pull request may close this issue.

spec: Add additional information about validator ordering (total power) cometbft/cometbft
2 participants
@adizere @andynog
2A96
0