Use ValidatorPublicKey instead of ValidatorName #3372

deuszx · 2025-02-19T14:39:57Z

Motivation

Currently we're using ValidatorName (a newtype wrapper around ValidatorPublicKey) in many places where it's not making any quality improvements. I'd argue that it's:

unnecessary (it's just a wrapper around our own struct)
confusing (we have ValidatorName and, next to it, ValidatorSignature that we verify with validator_name.public_key )
might clash (in the future) with "nicknames" - i.e. onchain human-readable aliases that validators may decide to register

Proposal

Replace usage of ValidatorName with ValidatorPublicKey. Also rename field names from name to public_key - this affects the RPC format and CLI.

Test Plan

CI should catch any regressions.

Release Plan

Nothing to do / These changes follow the usual release cycle.

Links

reviewer checklist

linera-faucet/server/src/lib.rs

linera-rpc/src/config.rs

linera-rpc/src/grpc/conversions.rs

CLI.md

linera-rpc/src/config.rs

linera-service/src/linera/main.rs

linera-service/src/server.rs

CLI.md

afck · 2025-02-20T10:16:19Z

linera-base/src/crypto/ed25519.rs

@@ -330,7 +330,7 @@ impl Ed25519Signature {
    pub fn verify_batch<'a, 'de, T, I>(value: &'a T, votes: I) -> Result<(), CryptoError>
    where
        T: BcsSignable<'de>,
-        I: IntoIterator<Item = (&'a Ed25519PublicKey, &'a Ed25519Signature)>,


Inside the ed25519 module I wouldn't use the alias.

Yes, found that in the next PR. Will push here.

Actually, I'm gonna do that in the follow-up PRs to avoid waiting for the CI again.

afck · 2025-02-20T10:18:37Z

linera-chain/src/data_types.rs

@@ -916,7 +916,7 @@ pub(crate) fn check_signatures(
    );
    // All that is left is checking signatures!
    let hash_and_round = VoteValue(value_hash, round, certificate_kind);
-    ValidatorSignature::verify_batch(&hash_and_round, signatures.iter().map(|(v, s)| (&v.0, s)))?;
+    ValidatorSignature::verify_batch(&hash_and_round, signatures.iter().map(|(v, s)| (v, s)))?;


Suggested change

ValidatorSignature::verify_batch(&hash_and_round, signatures.iter().map(|(v, s)| (v, s)))?;

ValidatorSignature::verify_batch(&hash_and_round, &signatures)?;

This won't work. The argument type is an iterator over references - not a reference to something that can be turned into an iterator.

Maybe verify_batch should actually accept an IntoIterator.

And even if not, wouldn't signatures.iter() suffice? Is the .map(|(v, s)| (v, s)) really necessary?

verify_batch already accepts IntoIterator.

Actually - it works 🤔 IDK why it didn't compile earlier.

afck · 2025-02-20T10:20:13Z

linera-chain/src/unit_tests/data_types_tests.rs


-    let committee = Committee::make_simple(vec![name1, name2]);
+    let committee = Committee::make_simple(vec![validator1_pk, validator2_pk]);


No need to introduce those two, since below you use v1.public_key anyway instead of validator1_pk.

Suggested change

let committee = Committee::make_simple(vec![validator1_pk, validator2_pk]);

let committee = Committee::make_simple(vec![key1.public(), key2.public()]);

afck

(No blockers from my side.)

deuszx force-pushed the refactor-use-pk-instead-of-owner branch from 2061f04 to c858a43 Compare February 19, 2025 14:46

deuszx requested review from afck, ma2bd and bart-linera and removed request for afck February 19, 2025 14:57

Use ValidatorPublicKey instead of ValidatorName

7471c40

deuszx force-pushed the refactor-use-pk-instead-of-owner branch from c858a43 to 7471c40 Compare February 19, 2025 15:05

deuszx marked this pull request as ready for review February 19, 2025 15:56

ma2bd reviewed Feb 19, 2025

View reviewed changes

linera-faucet/server/src/lib.rs Outdated Show resolved Hide resolved

ma2bd reviewed Feb 19, 2025

View reviewed changes

linera-rpc/src/config.rs Outdated Show resolved Hide resolved

ma2bd reviewed Feb 19, 2025

View reviewed changes

linera-rpc/src/grpc/conversions.rs Outdated Show resolved Hide resolved

ma2bd reviewed Feb 19, 2025

View reviewed changes

linera-rpc/src/grpc/conversions.rs Show resolved Hide resolved

deuszx force-pushed the refactor-use-pk-instead-of-owner branch from 1c45355 to 7bcec3b Compare February 19, 2025 16:53

Fix missed renames

b3e6534

deuszx force-pushed the refactor-use-pk-instead-of-owner branch from 7bcec3b to b3e6534 Compare February 19, 2025 16:54

deuszx added 3 commits February 19, 2025 17:13

Regenerate CLI.md

eb405cb

Fix integration tests

7b6ed01

Distinguish between validator and account keys in protobuf

16aacba

ma2bd reviewed Feb 19, 2025

View reviewed changes

CLI.md Outdated Show resolved Hide resolved

ma2bd reviewed Feb 19, 2025

View reviewed changes

linera-rpc/src/config.rs Outdated Show resolved Hide resolved

ma2bd reviewed Feb 19, 2025

View reviewed changes

linera-service/src/linera/main.rs Outdated Show resolved Hide resolved

ma2bd reviewed Feb 19, 2025

View reviewed changes

linera-service/src/server.rs Outdated Show resolved Hide resolved

CLI.md Outdated Show resolved Hide resolved

deuszx force-pushed the refactor-use-pk-instead-of-owner branch 3 times, most recently from 07d9ca9 to 3c76e1e Compare February 20, 2025 00:21

deuszx added 2 commits February 20, 2025 00:34

More renames

b974427

Bring back rm'd file

9293417

deuszx force-pushed the refactor-use-pk-instead-of-owner branch from 3c76e1e to 9293417 Compare February 20, 2025 00:34

Regenerate RPC format

8040897

afck reviewed Feb 20, 2025

View reviewed changes

afck approved these changes Feb 20, 2025

View reviewed changes

deuszx merged commit 840430a into main Feb 20, 2025
21 checks passed

deuszx deleted the refactor-use-pk-instead-of-owner branch February 20, 2025 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ValidatorPublicKey instead of ValidatorName #3372

Use ValidatorPublicKey instead of ValidatorName #3372

deuszx commented Feb 19, 2025 •

edited

Loading

afck Feb 20, 2025

deuszx Feb 20, 2025

deuszx Feb 20, 2025

afck Feb 20, 2025

deuszx Feb 20, 2025

afck Feb 20, 2025

deuszx Feb 20, 2025

deuszx Feb 20, 2025

afck Feb 20, 2025

afck left a comment

	ValidatorSignature::verify_batch(&hash_and_round, signatures.iter().map(\|(v, s)\| (v, s)))?;
	ValidatorSignature::verify_batch(&hash_and_round, &signatures)?;


		let committee = Committee::make_simple(vec![name1, name2]);
		let committee = Committee::make_simple(vec![validator1_pk, validator2_pk]);

Use ValidatorPublicKey instead of ValidatorName #3372

Use ValidatorPublicKey instead of ValidatorName #3372

Conversation

deuszx commented Feb 19, 2025 • edited Loading

Motivation

Proposal

Test Plan

Release Plan

Links

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afck left a comment

Choose a reason for hiding this comment

deuszx commented Feb 19, 2025 •

edited

Loading