Skip to content

Commit

Permalink
refactor(privval): rename sign_extension to skip_sign_extension (come…
Browse files Browse the repository at this point in the history
…tbft#2519)

because the former is not backwards compatible. I.e., if the remote
signer runs a new version for old chain and `sign_extension` field is
absent, then it will be false by default, BUT the expected behavior is
different. For this reason, the field is renamed to skip_sign_extension.
The remote signer might! skip signing extension bytes, but the default
behavior is to sign.

Thanks to @mzabaluev for spotting this.

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [x] Tests written/updated
- [x] 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
  • Loading branch information
melekes authored Mar 5, 2024
1 parent cb177fe commit 44ffd67
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 59 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
- `[privval]` DO NOT require extension signature from privval if vote
extensions are disabled. Remote signers must ONLY sign the extension if
`sign_extension` flag in `SignVoteRequest` is true.
extensions are disabled. Remote signers can skip signing the extension if
`skip_sign_extension` flag in `SignVoteRequest` is true.
[\#2496](https://github.com/cometbft/cometbft/pull/2496)
109 changes: 55 additions & 54 deletions api/cometbft/privval/v1/types.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion privval/signer_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (sc *SignerClient) GetPubKey() (crypto.PubKey, error) {

// SignVote requests a remote signer to sign a vote.
func (sc *SignerClient) SignVote(chainID string, vote *cmtproto.Vote, signExtension bool) error {
response, err := sc.endpoint.SendRequest(mustWrapMsg(&pvproto.SignVoteRequest{Vote: vote, ChainId: chainID, SignExtension: signExtension}))
response, err := sc.endpoint.SendRequest(mustWrapMsg(&pvproto.SignVoteRequest{Vote: vote, ChainId: chainID, SkipSignExtension: !signExtension}))
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion privval/signer_requestHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func DefaultValidationRequestHandler(

vote := r.SignVoteRequest.Vote

err = privVal.SignVote(chainID, vote, r.SignVoteRequest.SignExtension)
err = privVal.SignVote(chainID, vote, !r.SignVoteRequest.SkipSignExtension)
if err != nil {
res = mustWrapMsg(&pvproto.SignedVoteResponse{
Vote: cmtproto.Vote{}, Error: &pvproto.RemoteSignerError{Code: 0, Description: err.Error()},
Expand Down
2 changes: 1 addition & 1 deletion proto/cometbft/privval/v1/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ message PubKeyResponse {
message SignVoteRequest {
cometbft.types.v1.Vote vote = 1;
string chain_id = 2;
bool sign_extension = 3;
bool skip_sign_extension = 3; // if true, the signer may skip signing the extension bytes.
}

// SignedVoteResponse is a response containing a signed vote or an error
Expand Down

0 comments on commit 44ffd67

Please sign in to comment.