feat: add header and comet info service #15850
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
im thinking of defining the comet abci types for validatorUpdates and misbehaviour in golang so we can move it to core. how does this sound to others? |
core/blockinfo/service.go
Outdated
| GetDecidedLastCommit() CommitInfo // GetDecidedLastCommit returns the last commit info | ||
| } | ||
|
|
||
| // MisbehaviorType is the type of misbehavior for a validator |
There was a problem hiding this comment.
would love feedback on defining our own types like this
There was a problem hiding this comment.
Seems reasonable to me.
|
@tac0turtle your pull request is missing a changelog! |
github-prbot
requested review from
a team,
samricotta and
atheeshp
and removed request for
a team
core/blockinfo/service.go
Outdated
| GetDecidedLastCommit() CommitInfo // GetDecidedLastCommit returns the last commit info | ||
| } | ||
|
|
||
| // MisbehaviorType is the type of misbehavior for a validator |
There was a problem hiding this comment.
Seems reasonable to me.
core/comet/service.go
Outdated
| // CommitInfo is the commit information of ABCI | ||
| type CommitInfo interface { | ||
| Round() int32 | ||
| Votes() []VoteInfo |
There was a problem hiding this comment.
It would probably be better to avoid returning an array (which is actually mutable) and use a wrapper interface like the official protobuf does: https://pkg.go.dev/google.golang.org/protobuf/reflect/protoreflect#MessageDescriptors. Ex:
type VoteInfos interface {
Len() int
Get(int) VoteInfo
}This has the added performance benefit that we avoid allocating and copying an array when wrapping all the request types
| @@ -194,12 +194,14 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg | |||
| WithBlockGasMeter(gasMeter). | |||
| WithHeaderHash(req.Hash). | |||
| WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx)). | |||
| WithVoteInfos(req.LastCommitInfo.GetVotes()) | |||
| WithVoteInfos(req.LastCommitInfo.GetVotes()). | |||
| WithCometInfo(cometInfo{Misbehavior: req.ByzantineValidators, ValidatorsHash: req.Header.ValidatorsHash, ProposerAddress: req.Header.ProposerAddress, LastCommit: req.LastCommitInfo}) | |||
There was a problem hiding this comment.
why cometInfo vs a begin block request wrapper?
There was a problem hiding this comment.
because in the next pr integrating comet we drop the beginblock type, i was thinking might as well do this here
core/comet/service.go
Outdated
| GetCometInfo(context.Context) BlockInfo | ||
| } |
There was a problem hiding this comment.
How about we do a Misbehaviors wrapper like we did for VoteInfos instead of an array
| WithTxBytes(txBytes). | ||
| WithVoteInfos(app.voteInfos) | ||
| WithTxBytes(txBytes) | ||
| // WithVoteInfos(app.voteInfos) // TODO: identify if this is needed |
There was a problem hiding this comment.
Was this left commented intentionally?
There was a problem hiding this comment.
yes, its still unclear why this is needed, we can add it back via cometinfo to not cause such a large breaking change.
Co-authored-by: Aaron Craelius <[email protected]>
Description
ref #14683
this pr adds block info service as an interface and passes it to evidence.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change