Skip to content

Commit

Permalink
=Merge branch 'tomas/init-validator-own-keys' (#____)
Browse files Browse the repository at this point in the history
* origin/tomas/init-validator-own-keys:
  refactor sig verification gas handling
  fix `init_validator` bench
  changelog: add #2163
  client: store validator protocol key in the regular wallet
  wasm/tx_init_validator: check ownership of used keys with sigs
  ledger: add a tx host fn for sig verification
  client: sign tx_init_validator with all keys used

# Conflicts:
#	apps/src/lib/client/tx.rs
  • Loading branch information
Gianmarco Fraccaroli committed Nov 20, 2023
2 parents ec5837c + 88578bf commit f54b4b7
Show file tree
Hide file tree
Showing 18 changed files with 345 additions and 120 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Require to verify ownership of all validator keys when initialized on-chain.
([\#2163](https://github.com/anoma/namada/pull/2163))
10 changes: 5 additions & 5 deletions apps/src/lib/bench_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ impl Default for BenchShell {
bond,
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
);

bench_shell.execute_tx(&signed_tx);
Expand All @@ -264,7 +264,7 @@ impl Default for BenchShell {
},
None,
Some(vec![content_section]),
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
);

bench_shell.execute_tx(&signed_tx);
Expand Down Expand Up @@ -292,7 +292,7 @@ impl BenchShell {
data: impl BorshSerialize,
shielded: Option<Transaction>,
extra_sections: Option<Vec<Section>>,
signer: Option<&SecretKey>,
signers: Vec<&SecretKey>,
) -> Tx {
let mut tx =
Tx::from_type(namada::types::transaction::TxType::Decrypted(
Expand Down Expand Up @@ -321,7 +321,7 @@ impl BenchShell {
}
}

if let Some(signer) = signer {
for signer in signers {
tx.add_section(Section::Signature(Signature::new(
vec![tx.raw_header_hash()],
[(0, signer.clone())].into_iter().collect(),
Expand Down Expand Up @@ -894,7 +894,7 @@ impl BenchShieldedCtx {
},
shielded,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
}
}
36 changes: 32 additions & 4 deletions apps/src/lib/client/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ use rand::rngs::OsRng;
use super::rpc;
use crate::cli::{args, safe_exit};
use crate::client::rpc::query_wasm_code_hash;
use crate::client::tx::signing::{default_sign, SigningTxData};
use crate::client::tx::signing::{
default_sign, init_validator_signing_data, SigningTxData,
};
use crate::client::tx::tx::ProcessTxResponse;
use crate::config::TendermintMode;
use crate::facade::tendermint_rpc::endpoint::broadcast::tx_sync::Response;
Expand Down Expand Up @@ -504,7 +506,7 @@ pub async fn submit_init_validator<'a>(

let validator_key_alias = format!("{}-key", alias);
let consensus_key_alias = validator_consensus_key(&alias.clone().into());
// let consensus_key_alias = format!("{}-consensus-key", alias);
let protocol_key_alias = format!("{}-protocol-key", alias);

let threshold = match threshold {
Some(threshold) => threshold,
Expand Down Expand Up @@ -619,7 +621,25 @@ pub async fn submit_init_validator<'a>(
scheme,
)
.unwrap();
let protocol_key = validator_keys.get_protocol_keypair().ref_to();
let protocol_sk = validator_keys.get_protocol_keypair();
let protocol_key = protocol_sk.to_public();

// Store the protocol key in the wallet so that we can sign the tx with it
// to verify ownership
display_line!(namada.io(), "Storing protocol key in the wallet...");
let password = read_and_confirm_encryption_password(unsafe_dont_encrypt);
namada
.wallet_mut()
.await
.insert_keypair(
protocol_key_alias,
tx_args.wallet_alias_force,
protocol_sk.clone(),
password,
None,
None,
)
.map_err(|err| error::Error::Other(err.to_string()))?;

let validator_vp_code_hash =
query_wasm_code_hash(namada, validator_vp_code_path.to_str().unwrap())
Expand Down Expand Up @@ -688,9 +708,17 @@ pub async fn submit_init_validator<'a>(
validator_vp_code_hash: extra_section_hash,
};

// Put together all the PKs that we have to sign with to verify ownership
let mut all_pks = data.account_keys.clone();
all_pks.push(consensus_key.to_public());
all_pks.push(eth_cold_pk);
all_pks.push(eth_hot_pk);
all_pks.push(data.protocol_key.clone());

tx.add_code_from_hash(tx_code_hash).add_data(data);

let signing_data = aux_signing_data(namada, &tx_args, None, None).await?;
let signing_data =
init_validator_signing_data(namada, &tx_args, all_pks).await?;

tx::prepare_tx(
namada,
Expand Down
9 changes: 7 additions & 2 deletions benches/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn tx_section_signature_validation(c: &mut Criterion) {
transfer_data,
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
);
let section_hash = tx.header_hash();

Expand All @@ -48,7 +48,12 @@ fn tx_section_signature_validation(c: &mut Criterion) {
c.bench_function("tx_section_signature_validation", |b| {
b.iter(|| {
multisig
.verify_signature(&mut HashSet::new(), &pkim, &None, &mut None)
.verify_signature(
&mut HashSet::new(),
&pkim,
&None,
&mut || Ok(()),
)
.unwrap()
})
});
Expand Down
20 changes: 10 additions & 10 deletions benches/native_vps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ fn governance(c: &mut Criterion) {
},
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
}
"validator_vote" => {
Expand All @@ -104,7 +104,7 @@ fn governance(c: &mut Criterion) {
},
None,
None,
Some(&defaults::validator_keypair()),
vec![&defaults::albert_keypair()],
)
}
"minimal_proposal" => {
Expand All @@ -131,7 +131,7 @@ fn governance(c: &mut Criterion) {
},
None,
Some(vec![content_section]),
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
}
"complete_proposal" => {
Expand Down Expand Up @@ -183,7 +183,7 @@ fn governance(c: &mut Criterion) {
},
None,
Some(vec![content_section, wasm_code_section]),
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
}
_ => panic!("Unexpected bench test"),
Expand Down Expand Up @@ -423,7 +423,7 @@ fn vp_multitoken(c: &mut Criterion) {
},
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
);

for (signed_tx, bench_name) in [foreign_key_write, transfer]
Expand Down Expand Up @@ -624,7 +624,7 @@ fn pgf(c: &mut Criterion) {
defaults::albert_address(),
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
),
"steward_inflation_rate" => {
let data =
Expand All @@ -640,7 +640,7 @@ fn pgf(c: &mut Criterion) {
data,
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
}
_ => panic!("Unexpected bench test"),
Expand Down Expand Up @@ -712,7 +712,7 @@ fn eth_bridge_nut(c: &mut Criterion) {
data,
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
};

Expand Down Expand Up @@ -781,7 +781,7 @@ fn eth_bridge(c: &mut Criterion) {
data,
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
};

Expand Down Expand Up @@ -878,7 +878,7 @@ fn eth_bridge_pool(c: &mut Criterion) {
data,
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
)
};

Expand Down
2 changes: 1 addition & 1 deletion benches/process_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn process_tx(c: &mut Criterion) {
},
None,
None,
Some(&defaults::albert_keypair()),
vec![&defaults::albert_keypair()],
);

tx.update_header(namada::types::transaction::TxType::Wrapper(Box::new(
Expand Down
Loading

0 comments on commit f54b4b7

Please sign in to comment.