Skip to content

docs: update the privval extension CIP to match the expected implementation #342

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

Merged
cmwaters merged 3 commits into from
Aug 1, 2025
Merged

docs: update the privval extension CIP to match the expected implementation #342

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2ff05f
docs: update the privval extension CIP to match the expected implemen…
rach-id Jul 31, 2025
be464a0
chore: apply feedback review
rach-id Jul 31, 2025
8b97398
chore: use marshal delimited and update status to review
rach-id Aug 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 53 additions & 30 deletions cips/cip-040.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
| cip | 40 |
| - | - |
| title | Privval Interface Extension for Arbitrary Message Signing |
| description | Extends the CometBFT privval interface to support signing arbitrary messages for offchain protocols. |
| author | CHAMI Rachid ([@rach-id](https://github.com/rach-id)), Evan Forbes ([evan-forbes](https://github.com/evan-forbes)) |
| discussions-to | <https://forum.celestia.org/t/cip-40-privval-interface-extension-for-arbitrary-message-signing/2102> |
| status | Draft |
| type | Standards Track |
| category | Interface |
| created | 2025-07-25 |
| cip | 40 |
|----------------|--------------------------------------------------------------------------------------------------------------------|
| title | Privval Interface Extension for Arbitrary Message Signing |
| description | Extends the CometBFT privval interface to support signing arbitrary messages for offchain protocols. |
| author | CHAMI Rachid ([@rach-id](https://github.com/rach-id)), Evan Forbes ([evan-forbes](https://github.com/evan-forbes)) |
| discussions-to | <https://forum.celestia.org/t/cip-40-privval-interface-extension-for-arbitrary-message-signing/2102> |
| status | Review |
| type | Standards Track |
| category | Interface |
| created | 2025-07-25 |

## Abstract

Expand All @@ -33,7 +33,7 @@ The following message types SHALL be added to the `privval.Message` interface:
message SignRawBytesRequest {
string chain_id = 1;
bytes raw_bytes = 2;
uint32 unique_id = 3;
string unique_id = 3;
}

message SignedRawBytesResponse {
Expand Down Expand Up @@ -66,37 +66,60 @@ message Message {

### Field Specifications

- `chain_id`: The chain identifier to prevent cross-chain signature reuse.
- `raw_bytes`: The digest that is signed over. This can be any data that the consensus node includes, however note that the actual bytes signed over MUST be constructed as `chain_id + unique_id + raw_bytes`.
- `unique_id`: A required uint32 identifier for the specific protocol or message type being signed.
- `chain_id`: The chain identifier to prevent cross-chain signature reuse. It's required as it's used in signing and also routing in KMS implementation.
- `raw_bytes`: It's the data that needs to be signed over. Worth noting that this shouldn't be a digest, it needs to be the actual data, and it's a required field. The sign bytes are constructed as defined in the [sign bytes construction](#sign-bytes-construction) section.
- `unique_id`: A required string identifier for the specific protocol or message type being signed.
- `signature`: The resulting signature bytes from the signing operation.
- `error`: Error information if the signing operation fails.

### Sign Bytes Construction

The actual bytes that are signed MUST be constructed by concatenating:

```text
sign_bytes = chain_id + unique_id + raw_bytes
The actual bytes that are signed MUST be constructed by concatenating the domain separator `"COMET::RAW_BYTES::SIGN"` with the protobuf encoding of the `SignRawBytesRequest`:

```go
// RawBytesSignBytesPrefix defines a domain separator prefix added to raw bytes to ensure the resulting
// signed message can't be confused with a consensus message, which could lead to double signing
const RawBytesSignBytesPrefix = "COMET::RAW_BYTES::SIGN"

// RawBytesMessageSignBytes returns the canonical bytes for signing raw data messages.
// It requires non-empty chainID, uniqueID, and rawBytes to prevent security issues.
// Returns error if any required parameter is empty or if marshaling fails.
func RawBytesMessageSignBytes(chainID, uniqueID string, rawBytes []byte) ([]byte, error) {
if chainID == "" {
return nil, errors.New("chainID cannot be empty")
}

if uniqueID == "" {
return nil, fmt.Errorf("uniqueID cannot be empty")
}

if len(rawBytes) == 0 {
return nil, fmt.Errorf("rawBytes cannot be empty")
}

prefix := []byte(RawBytesSignBytesPrefix)

signRequest := &privval.SignRawBytesRequest{
Copy link
Collaborator

@cmwaters cmwaters Aug 1, 2025

Choose a reason for hiding this comment

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

Just a semantic nit, it would be clearer to have the prefix as part of the SignRawBytesRequest, so a user can easily see all the fields that get marshalled and won't miss the prefix that gets prepended at the end

Copy link
Member Author

@rach-id rach-id Aug 1, 2025

Choose a reason for hiding this comment

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

I see, but this request is going on the wire, I don't see a use for a field that's going to be always the same value to be added on each request.

Copy link
Collaborator

@cmwaters cmwaters Aug 1, 2025

Choose a reason for hiding this comment

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

It was just to indicate that there is a prefix and that the sign raw bytes isn't all of the signed raw bytes. As I said this is a semantic nit

ChainId: chainID,
RawBytes: rawBytes,
UniqueId: uniqueID,
}
protoBytes, err := protoio.MarshalDelimited(signRequest)
if err != nil {
return nil, err
}
return append(prefix, protoBytes...), nil
}
```

### Encoding Specifications

For sign bytes construction, each component MUST be encoded as UTF-8 byte sequences:

- `chain_id`: UTF-8 encoded string bytes (e.g., "celestia" → [0x63, 0x65, 0x6c, 0x65, 0x73, 0x74, 0x69, 0x61, 0x2d, 0x6d, 0x61, 0x69, 0x6e, 0x6e, 0x65, 0x74])
- `unique_id`: Decimal string representation of uint32 value encoded as UTF-8 bytes (e.g., uint32(123) → "123" → [0x31, 0x32, 0x33])
- `raw_bytes`: Raw byte sequence as-is, no additional encoding

### Implementation Requirements

1. KMS implementations MUST support the new message types for full compatibility.
2. The signing operation MUST use the same cryptographic key as consensus message signing.
3. The chain_id field MUST match the configured chain identifier.
4. Double-signing protection is NOT REQUIRED for raw message signing operations.
5. Convert each component to its byte representation as specified above
6. Concatenate the byte sequences directly without delimiters
7. Sign the resulting byte array
5. Generate the sign bytes as per the [sign bytes construction](#sign-bytes-construction) section.
6. Sign the resulting byte array.

## Rationale

Expand All @@ -114,7 +137,7 @@ For sign bytes construction, each component MUST be encoded as UTF-8 byte sequen

## Backwards Compatibility

This proposal is fully backwards compatible. Existing KMS implementations will continue to function normally, as the new message types use previously unused field numbers in the protobuf oneof union. That being said, all repos that import the interface MUST update their implementions to at least use a noop.
This proposal is fully backwards compatible. Existing KMS implementations will continue to function normally, as the new message types use previously unused field numbers in the protobuf oneof union. That being said, all repos that import the interface MUST update their implementations to at least use a noop.

## Security Considerations

Expand Down