Skip to content
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.

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

Use secp256k1 for validator keys. #3377

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion CLI.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,11 +404,12 @@ Synchronizes a validator with the local state of chains

Add or modify a validator (admin only)

**Usage:** `linera set-validator [OPTIONS] --public-key <PUBLIC_KEY> --address <ADDRESS>`
**Usage:** `linera set-validator [OPTIONS] --public-key <PUBLIC_KEY> --account-key <ACCOUNT_KEY> --address <ADDRESS>`

###### **Options:**

* `--public-key <PUBLIC_KEY>` — The public key of the validator
* `--account-key <ACCOUNT_KEY>` — The public key of the account controlled by the validator
Comment on lines 411 to +412
Copy link
Contributor

@ma2bd ma2bd Feb 22, 2025

Choose a reason for hiding this comment

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

I would expect --xxx-public-key in both cases for some value xxx.

Couple of naming options: (key = private or public key depending on the context)

  • validator key vs user key
  • validator key vs owner key
  • (validator's) server key vs (validator's) client key <-- seems like the most precise because both keys belong to the validator node.

What do you think @deuszx @afck ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, but let's do the renaming in a single sweep in a separate PR. This one is already too big and it (rename) is orthogonal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, this works.

For the records, ideally, you would start by renaming ValidatorState::public_key into something more specific like ValidatorState::server_public_key in order to "make space". Then this PR would add (say) ValidatorState::client_public_key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially it was "authority key" which I think made sense (since validator can have many keys but only one is signing blocks).

Copy link
Contributor

@ma2bd ma2bd Feb 22, 2025

Choose a reason for hiding this comment

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

"authority key" is nice. Something like: "validator authority key" vs "validator user/owner/client key" then

* `--address <ADDRESS>` — Network address
* `--votes <VOTES>` — Voting power

Expand Down
29 changes: 29 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ serde_yaml = "0.8.26"
serde-name = "0.2.1"
serde-reflection = "0.3.6"
serde-wasm-bindgen = "0.6.5"
serde_with = { version = "3", default-features = false, features = ["alloc", "macros"] }
sha3 = "0.10.8"
similar-asserts = "1.5.0"
static_assertions = "1.1.0"
Expand Down
70 changes: 70 additions & 0 deletions examples/Cargo.lock

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

2 changes: 2 additions & 0 deletions linera-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ serde.workspace = true
serde-name.workspace = true
serde_bytes.workspace = true
serde_json.workspace = true
serde_with.workspace = true
test-strategy = { workspace = true, optional = true }
thiserror.workspace = true
tokio = { workspace = true, features = ["time"] }
Expand Down Expand Up @@ -79,6 +80,7 @@ port-selector.workspace = true
zstd.workspace = true

[dev-dependencies]
bcs.workspace = true
linera-base = { path = ".", default-features = false, features = ["test"] }
linera-witty = { workspace = true, features = ["test"] }
test-case.workspace = true
Expand Down
5 changes: 3 additions & 2 deletions linera-base/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,10 @@ impl CommandExt for tokio::process::Command {
.with_context(|| self.description())?;
ensure!(
output.status.success(),
"{}: got non-zero error code {}",
"{}: got non-zero error code {}. Stderr: \n{:?}\n",
self.description(),
output.status
output.status,
String::from_utf8(output.stderr),
);
String::from_utf8(output.stdout).with_context(|| self.description())
}
Expand Down
47 changes: 26 additions & 21 deletions linera-base/src/crypto/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ impl<'de> Deserialize<'de> for Ed25519PublicKey {
} else {
#[derive(Deserialize)]
#[serde(rename = "Ed25519PublicKey")]
struct Foo([u8; dalek::PUBLIC_KEY_LENGTH]);
struct PublicKey([u8; dalek::PUBLIC_KEY_LENGTH]);

let value = Foo::deserialize(deserializer)?;
let value = PublicKey::deserialize(deserializer)?;
Ok(Self(value.0))
}
}
Expand Down Expand Up @@ -288,23 +288,6 @@ impl Ed25519Signature {
})
}

/// Checks an optional signature.
pub fn check_optional_signature<'de, T>(
signature: Option<&Self>,
value: &T,
author: &Ed25519PublicKey,
) -> Result<(), CryptoError>
where
T: BcsSignable<'de> + fmt::Debug,
{
match signature {
Some(sig) => sig.check(value, *author),
None => Err(CryptoError::MissingSignature {
type_name: T::type_name().to_string(),
}),
}
}

fn verify_batch_internal<'a, 'de, T, I>(
value: &'a T,
votes: I,
Expand Down Expand Up @@ -368,9 +351,9 @@ impl<'de> Deserialize<'de> for Ed25519Signature {
} else {
#[derive(Deserialize)]
#[serde(rename = "Ed25519Signature")]
struct Foo(dalek::Signature);
struct Signature(dalek::Signature);

let value = Foo::deserialize(deserializer)?;
let value = Signature::deserialize(deserializer)?;
Ok(Self(value.0))
}
}
Expand Down Expand Up @@ -435,4 +418,26 @@ mod tests {
assert!(s.check(&tsx, addr1).is_err());
assert!(s.check(&foo, addr1).is_err());
}

#[test]
fn test_public_key_serialization() {
use crate::crypto::ed25519::Ed25519PublicKey;
let key_in = Ed25519PublicKey::test_key(0);
let s = serde_json::to_string(&key_in).unwrap();
let key_out: Ed25519PublicKey = serde_json::from_str(&s).unwrap();
assert_eq!(key_out, key_in);

let s = bcs::to_bytes(&key_in).unwrap();
let key_out: Ed25519PublicKey = bcs::from_bytes(&s).unwrap();
assert_eq!(key_out, key_in);
}

#[test]
fn test_secret_key_serialization() {
use crate::crypto::ed25519::Ed25519SecretKey;
let key_in = Ed25519SecretKey::generate();
let s = serde_json::to_string(&key_in).unwrap();
let key_out: Ed25519SecretKey = serde_json::from_str(&s).unwrap();
assert_eq!(key_out.0.to_bytes(), key_in.0.to_bytes());
}
}
12 changes: 7 additions & 5 deletions linera-base/src/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ use serde::{Deserialize, Serialize};
use thiserror::Error;

/// The public key of a validator.
pub type ValidatorPublicKey = ed25519::Ed25519PublicKey;
pub type ValidatorPublicKey = secp256k1::Secp256k1PublicKey;
/// The private key of a validator.
pub type ValidatorSecretKey = ed25519::Ed25519SecretKey;
pub type ValidatorSecretKey = secp256k1::Secp256k1SecretKey;
/// The signature of a validator.
pub type ValidatorSignature = ed25519::Ed25519Signature;
pub type ValidatorSignature = secp256k1::Secp256k1Signature;
/// The key pair of a validator.
pub type ValidatorKeypair = secp256k1::Secp256k1KeyPair;

/// The public key of a chain owner.
/// The corresponding private key is allowed to propose blocks
Expand All @@ -38,8 +40,8 @@ pub type AccountSignature = ed25519::Ed25519Signature;
pub enum CryptoError {
#[error("Signature for object {type_name} is not valid: {error}")]
InvalidSignature { error: String, type_name: String },
#[error("Signature for object {type_name} is missing")]
MissingSignature { type_name: String },
#[error("Signature from validator is missing")]
MissingValidatorSignature,
#[error(transparent)]
NonHexDigits(#[from] hex::FromHexError),
#[error(
Expand Down
Loading