NDRS-149: Read the validator's secret key from disk by EdHastingsCasperAssociation · Pull Request #95 · casper-network/casper-node

EdHastingsCasperAssociation · 2020-07-17T07:50:56Z

This PR retrieves the signing key for a validator from disk for use by consensus. The consensus component did not have a config section with which to contain this value; it does now.

Additionally, to allow ease of use when running multiple nodes to test a network with, support for providing overrides for values in config files via command line is also contained in this PR. The README.md has been updated with an example of how to use this functionality.

The chainspec.toml path was being passed to the initializer as part of a tuple; the value is moved into the config file as part of this PR. Using the above mentioned configured value override functionality, the default configured value can be overridden via command line if desired.

https://casperlabs.atlassian.net/browse/NDRS-149

Note: some example pem files are included which correspond (in positional order) to example public keys contained in an example accounts.csv file; they should only be used for testing or demonstration purposes.

node/src/app/cli.rs

resources/local/config.toml

node/src/app/cli.rs

afck · 2020-07-17T10:24:44Z

node/src/app/cli.rs

+    /// `section.key=value`
+    fn from_str(input: &str) -> Result<Self, Self::Err> {
+        let re = Regex::new(r"^([^.]+)\.([^=]+)=(.+)$").unwrap();
+        if let Some(captures) = re.captures(input) {


You could return early here:

let captures = re.captures(input).ok_or("could not parse config_ext (see README.md)")?;

afck · 2020-07-17T10:36:50Z

node/src/crypto/asymmetric_key.rs

+        match Self::ed25519_from_bytes(pem.contents) {
+            Ok(secret_key) => Ok(Some(secret_key)),
+            Err(error) => Err(error),
+        }


Maybe more concise:

Self::ed25519_from_bytes(pem.contents).map(Some)

Fraser999

We should probably specify somewhere in the README that paths provided in the config file must either be absolute, or relative to the config file itself.

No blockers though, so optimistic approval assuming that invalid README examples are updated.

Fraser999 · 2020-07-17T10:06:39Z

README.md

+cargo run --release -- validator -p=resources/local/chainspec.toml -c=config.toml
+```
+
+If using a config file as described above, it is also possible to override and extend the values in the config file


I think we changed that so it doesn't only apply when using a config file. We could update this to say something more like:

It is also possible to specify individual config file options from the command line using one or more args in the form of `-C <section>.<key>=<value>`. These will override values set in a config file if provided, or will override the default values otherwise.

README.md

Fraser999 · 2020-07-17T10:12:08Z

README.md


 ```
-cargo run --release -- validator -p=resources/example/chainspec.toml
+cargo run --release -- validator -p=resources/local/chainspec.toml


Suggested change

cargo run --release -- validator -p=resources/local/chainspec.toml

cargo run --release -- validator -c=resources/local/config.toml

README.md

Fraser999 · 2020-07-17T10:19:47Z

README.md


-As well as the [example chainspec](resources/example/chainspec.toml), there is a commented
-[example config file](resources/example/config.toml) in the same folder.
+As well as the [local chainspec](resources/local/chainspec.toml), there is a commented


We can probably remove these lines altogether now, since we refer to the config file in the first example block above?

Fraser999 · 2020-07-17T10:29:07Z

node/src/app/cli.rs

+
+impl ConfigExt {
+    /// Updates toml table with updated or extended kvp.
+    fn update_toml_table(&self, toml_value: &mut toml::Value) -> Option<()> {


I suppose we should bring toml::value::Table into scope so we don't need to fully qualify it everywhere. And toml::Value is already in scope, so we don't need to fully qualify it everywhere in this file.

fizyk20

I thought that actually passing the secret key to the consensus component will fall onto me as a part of my task, but I'm happy to see that it's done here already 😄

node/src/crypto/asymmetric_key.rs

SyncLeap checks, deploy buffer purge, cleanups

95: Restart current round after unpausing. r=afck a=afck This resets `self.current_timeout` after unpausing. The effect is both that the current round's proposer immediately creates another proposal (unless they've already proposed), and that the other validators restart their timeout, so there is a chance the proposal will get accepted. In nctl tests with block time 0 the pause mode is often activated and deactivated in quick succession, which caused a lot of proposals getting dropped. This change fixes that problem. Closes https://github.com/casper-network/Governance/issues/342. Co-authored-by: Andreas Fackler <andreas@casperlabs.io>

* split fuel-tx into parallel jobs * fix clippy errors

We often need to convert directly from a panic reason to an io error, such as when performing to/from bytes encoding - in that case, the VM will not be intermediary to the operation and the conversion will be performed directly. This commit introduces a transparent conversion between the error types.

NDRS-149: Read the validator's secret key from disk

88616bc

EdHastingsCasperAssociation requested review from Fraser999, fizyk20 and marc-casperlabs July 17, 2020 07:50

EdHastingsCasperAssociation assigned fizyk20 Jul 17, 2020

NDRS-149: correct typos

aac2877

mpapierski reviewed Jul 17, 2020

View reviewed changes

node/src/app/cli.rs Show resolved Hide resolved

resources/local/config.toml Show resolved Hide resolved

afck approved these changes Jul 17, 2020

View reviewed changes

Fraser999 approved these changes Jul 17, 2020

View reviewed changes

fizyk20 approved these changes Jul 17, 2020

View reviewed changes

node/src/crypto/asymmetric_key.rs Show resolved Hide resolved

Fraser999 merged commit a2867d1 into casper-network:master Jul 17, 2020

Fraser999 mentioned this pull request Jul 20, 2020

NDRS-149: address review comments #101

Merged

EdHastingsCasperAssociation deleted the NDRS-149 branch July 29, 2020 20:11

rafal-ch pushed a commit to rafal-ch/casper-node that referenced this pull request Nov 2, 2022

Merge pull request casper-network#95 from afck/fast-sync-v2

9f93f4c

SyncLeap checks, deploy buffer purge, cleanups

rafal-ch pushed a commit that referenced this pull request Sep 11, 2024

0.5.1 version bump (#95)

2f4f570

rafal-ch pushed a commit that referenced this pull request Sep 11, 2024

split fuel-tx CI into parallel jobs (#95)

beda125

* split fuel-tx into parallel jobs * fix clippy errors

EdHastingsCasperAssociation added a commit to EdHastingsCasperAssociation/casper-node that referenced this pull request Oct 9, 2024

fixes casper-network#95 (probably)

547afcc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NDRS-149: Read the validator's secret key from disk#95

NDRS-149: Read the validator's secret key from disk#95
Fraser999 merged 2 commits intocasper-network:masterfrom
EdHastingsCasperAssociation:NDRS-149

EdHastingsCasperAssociation commented Jul 17, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

afck Jul 17, 2020

Uh oh!

afck Jul 17, 2020

Uh oh!

Fraser999 left a comment

Uh oh!

Fraser999 Jul 17, 2020

Uh oh!

Uh oh!

Fraser999 Jul 17, 2020

Uh oh!

Uh oh!

Uh oh!

Fraser999 Jul 17, 2020

Uh oh!

Fraser999 Jul 17, 2020

Uh oh!

fizyk20 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	cargo run --release -- validator -p=resources/local/chainspec.toml
	cargo run --release -- validator -c=resources/local/config.toml

Conversation

EdHastingsCasperAssociation commented Jul 17, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

afck Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

afck Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

Fraser999 left a comment

Choose a reason for hiding this comment

Uh oh!

Fraser999 Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Fraser999 Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Fraser999 Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

Fraser999 Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

fizyk20 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants