8000 Proposal: Extend the `privval.Message` interface to support signing arbitrary hashes · Issue #5126 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Proposal: Extend the privval.Message interface to support signing arbitrary hashes #5126

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
Open
rach-id opened this issue May 14, 2025 · 2 comments
Open

Proposal: Extend the privval.Message interface to support signing arbitrary hashes #5126

rach-id opened this issue May 14, 2025 · 2 comments
Labels
enhancement New feature or request needs-triage This issue/PR has not yet been triaged by the team.

Comments

@rach-id
Copy link
rach-id commented May 14, 2025

Feature Request

Summary

Celestia is introducing high throughput and low latency gossiping protocols that rely on the validators being able to commit to offchain messages using their consensus keys and existing KMS. However, the current key management systems only support signing predefined consensus messages via the privval.Message interface. To address this limitation, this proposal extends that interface to allow signing arbitrary message hashes in a backwards compatible way.

It’s worth noting that signing hashes of offchain messages doesn’t require the typical “watermark” double signing protection.

Details

More specifically, Celestia is adding two protocols that would make use of this feature. The first is what we’re calling the Full Mesh Overlay (FMO), in which all validators directly connect to each other alongside their normal connections to consensus nodes. The second is what we’re referring to as Vacuum!, in which validators are committing to blobs that they have before they are included in a block.

Proposal

Include the following types in the privval.Message interface:

message SignP2PMessageRequest {
  bytes  hash      = 1;
  string chain_id  = 2;
  string unique_id = 3;
}

message SignedP2PMessageResponse {
  bytes             signature = 1;
  RemoteSignerError error     = 2;
}

So the Message interface will become:

message Message {
  oneof sum {
    PubKeyRequest            pub_key_request             = 1;
    PubKeyResponse           pub_key_response            = 2;
    SignVoteRequest          sign_vote_request           = 3;
    SignedVoteResponse       signed_vote_response        = 4;
    SignProposalRequest      sign_proposal_request       = 5;
    SignedProposalResponse   signed_proposal_response    = 6;
    PingRequest              ping_request                = 7;
    PingResponse             ping_response               = 8;
    // new types: 
    SignP2PMessageRequest    sign_p2p_message_request    = 9;
    SignedP2PMessageResponse signed_p2p_message_response = 10;
  }
}

Pros

  • Native support for signing arbitrary offchain messages.
  • Lay the groundwork for other consumers of the CometBFT and Malachite stacks to implement modern low latency high throughput gossiping mechanisms.
  • Backwards compatible.

Security risks

Double-signing protection isn’t necessary for signing p2p messages in almost all cases. To simplify the implementation for the many kms we’re suggesting to explicitly not add it.
@rach-id rach-id added enhancement New feature or request needs-triage This issue/PR has not yet been triaged by the team. labels May 14, 2025
@crStiv crStiv mentioned this issue May 16, 2025
Closed
3 tasks
@rach-id rach-id mentioned this issue May 19, 2025
Draft
@tony-iqlusion
Copy link

What is the actual body of the message to be signed in this case, and what specifically makes it a "P2P" message?

Note that Ed25519, which is typically used for consensus keys, doesn't support the signing of arbitrary message digests the same way e.g. ECDSA does. Is the hash going to be enveloped in some other message before signing, e.g. a serialized proto? Domain separation with other consensus messages would be a nice property too.

@rach-id
Copy link
Author
rach-id commented May 20, 2025

What is the actual body of the message to be signed in this case, and what specifically makes it a "P2P" message?

Nothing makes it a P2P message. Given this confusion, we changed the name to Digest and these are the new types: https://github.com/rach-id/cometbft/blob/6749b21a5eeb7655db607c25a971bc68666fc549/proto/tendermint/privval/types.proto#L65-L74

Also, that branch contains the changes that we typically want to push upstream.

Would it be better to update the issue also to reflect the new types names?

Note that Ed25519, which is typically used for consensus keys, doesn't support the signing of arbitrary message digests the same way e.g. ECDSA does. Is the hash going to be enveloped in some other message before signing, e.g. a serialized proto? Domain separation with other consensus messages would be a nice property too.

That's a very good point. Actually, the hash, which we renamed to digest can be the output of any hashing function. But given this property, I guess it would be better to change it to raw bytes. However, as you can see in the implementation, we don't apply any constraints on the provided data, so we can just rename the field.

Also, the sign bytes are just the concatenation of the fields: https://github.com/rach-id/cometbft/blob/6749b21a5eeb7655db607c25a971bc68666fc549/types/priv_validator.go#L24-L27

// DigestSignBytes returns the bytes to sign for a digest message
func DigestSignBytes(chainID, uniqueID string, hash cmtbytes.HexBytes) []byte {
	return []byte(chainID + uniqueID + hash.String())
}

So we're already providing a chainID and a uniqueID that can be changed depending on the context. Also, additional domain separation can be added to something like:

"COMET::RAW_DATA::SIGN|" + chainID + "|" + uniqueID + "|" + raw_data

What do you think?

@rach-id rach-id mentioned this issue May 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request needs-triage This issue/PR has not yet been triaged by the team.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rach-id @tony-iqlusion
0