ETF MILESTONE 2 #1

driemworks · 2024-04-16T02:31:55Z

No description provided.

polkadot/runtime/rococo/src/lib.rs

polkadot/runtime/westend/src/lib.rs

substrate/primitives/core/src/crypto_bytes.rs

juangirini · 2024-04-17T13:45:33Z

substrate/bin/node/cli/src/chain_spec.rs

+	initial_authorities.iter().enumerate().map(|(idx, auth)| {
+		let pok = &genesis_resharing[idx].1;
+		let mut bytes = Vec::new();
+		pok.serialize_compressed(&mut bytes).unwrap();


Do we need to unwrap this?

We do. It could be an expect instead, but it's fine if this panics since it's a critical failure if the pok can't be serialized on genesis anyway.

substrate/client/consensus/beefy/src/communication/gossip.rs

substrate/client/consensus/beefy/src/keystore.rs

juangirini · 2024-04-17T13:51:43Z

substrate/client/consensus/beefy/src/lib.rs

@@ -342,6 +343,7 @@ where
 		let mut sessions = VecDeque::new();
 		let mut header = best_grandpa.clone();
 		let state = loop {
+


This is just a nit, but in general, we should avoid unnecessary diff changes

Suggested change

juangirini · 2024-04-17T13:52:54Z

substrate/client/consensus/beefy/src/worker.rs

@@ -76,13 +83,13 @@ pub(crate) enum RoundAction {
 pub(crate) struct VoterOracle<B: Block> {
 	/// Queue of known sessions. Keeps track of voting rounds (block numbers) within each session.
 	///
-	/// There are three voter states corresponding to three queue states:
+	/// There are three voter states coresponding to three queue states:


double r is correct

juangirini · 2024-04-17T13:57:43Z

substrate/frame/beefy/src/lib.rs

@@ -444,6 +419,52 @@ impl<T: Config> Pallet<T> {

 		Ok(())
 	}
+
+	fn initialize_genesis_shares(


I guess it is fine for this fn to panic?

juangirini · 2024-04-22T08:33:51Z

substrate/bin/node/cli/src/chain_spec.rs

+			// EB::SignatureGroup::deserialize_compressed(
+			// 	// [48 bytes for SigGroup][96 bytes for PubKeyGroup]
+			// 	&authority.to_raw_vec()[..48]
+			// ).unwrap()


Suggested change

// EB::SignatureGroup::deserialize_compressed(

// // [48 bytes for SigGroup][96 bytes for PubKeyGroup]

// &authority.to_raw_vec()[..48]

// ).unwrap()

juangirini · 2024-04-22T08:34:08Z

substrate/bin/node/cli/src/chain_spec.rs

+			// authority_keys_from_seed("Bob"), 
+			// authority_keys_from_seed("Charlie")


Suggested change

// authority_keys_from_seed("Bob"),

// authority_keys_from_seed("Charlie")

juangirini · 2024-04-22T08:34:57Z

substrate/client/consensus/beefy/Cargo.toml

 sp-mmr-primitives = { path = "../../../primitives/merkle-mountain-range" }
 sp-runtime = { path = "../../../primitives/runtime" }
 tokio = "1.22.0"
+# etf-crypto-primitives = { git = "https://github.com/ideal-lab5/etf-sdk.git", branch = "w3fbls-migration" }


juangirini · 2024-04-22T08:36:34Z

substrate/bin/node/cli/Cargo.toml

@@ -118,6 +118,15 @@ node-inspect = { package = "staging-node-inspect", path = "../inspect", optional
 try-runtime-cli = { path = "../../../utils/frame/try-runtime/cli", optional = true }
 serde_json = { workspace = true, default-features = true }

+# etf dependencies
+etf-crypto-primitives = { git = "https://github.com/ideal-lab5/etf-sdk.git", branch = "w3fbls-migration" }


juangirini · 2024-04-22T09:19:23Z

substrate/client/consensus/beefy/src/communication/gossip.rs

+
+		// recover the signature bytes from the payload
+		let raw_etf_payload = vote.commitment.payload.get_raw(&sp_consensus_beefy::known_payloads::ETF_SIGNATURE)
+			.unwrap_or({


juangirini · 2024-04-22T09:19:31Z

substrate/client/consensus/beefy/src/communication/gossip.rs


-		if BeefyKeystore::verify(&vote.id, &vote.signature, &vote.commitment.encode()) {
+		if BeefyKeystore::verify(&vote.id, &vote.signature, &vote.commitment.encode())
+			// && BeefyKeystore::verify(&vote.id, &etf_sig, &round.to_string().as_bytes()) 


Suggested change

// && BeefyKeystore::verify(&vote.id, &etf_sig, &round.to_string().as_bytes())

juangirini · 2024-04-22T09:20:39Z

substrate/client/consensus/beefy/src/keystore.rs

+				let sig = store
+					.bls377_sign(BEEFY_KEY_TYPE, &public, &message)
+					.map_err(|e| error::Error::Keystore(e.to_string()))?
+					.ok_or_else(|| error::Error::Signature("bls377_sign()  failed".to_string()))?;


Suggested change

.ok_or_else(|| error::Error::Signature("bls377_sign() failed".to_string()))?;

.ok_or_else(|| error::Error::Signature("bls377_sign() failed".to_string()))?;

juangirini · 2024-04-22T09:23:45Z

substrate/client/consensus/beefy/src/worker.rs

+		// // `target` is guaranteed > `best_beefy` since `min_block_delta` is at least `1`.
+		// let target =
+		// 	vote_target(best_grandpa, best_beefy, rounds.session_start(), self.min_block_delta);
+		// info!(
+		// 	target: LOG_TARGET,
+		// 	"🥩 best beefy: #{:?}, best finalized: #{:?}, current_vote_target: {:?}",
+		// 	best_beefy,
+		// 	best_grandpa,
+		// 	target
+		// );
+		// target


cleanup

Suggested change

// // `target` is guaranteed > `best_beefy` since `min_block_delta` is at least `1`.

// let target =

// vote_target(best_grandpa, best_beefy, rounds.session_start(), self.min_block_delta);

// info!(

// target: LOG_TARGET,

// "🥩 best beefy: #{:?}, best finalized: #{:?}, current_vote_target: {:?}",

// best_beefy,

// best_grandpa,

// target

// );

// target

juangirini · 2024-04-22T09:39:40Z

substrate/client/consensus/beefy/src/worker.rs

+		if let Some(Some(validator_set)) = runtime_api.validator_set(hash).ok() {
+			if let Some(Some(pok_bytes)) = runtime_api.read_share(hash, id.clone()).ok() {
+				if let Ok(sig)  = self.key_store.etf_sign(
+					&id, 
+					&pok_bytes, 
+					&message, validator_set.len() as u8
+				) {
+					return Some(sig);
+				}
+			}
+		}


Optional: I usually find match statements easier to read than nested ifs

Suggested change

if let Some(Some(validator_set)) = runtime_api.validator_set(hash).ok() {

if let Some(Some(pok_bytes)) = runtime_api.read_share(hash, id.clone()).ok() {

if let Ok(sig) = self.key_store.etf_sign(

&id,

&pok_bytes,

&message, validator_set.len() as u8

) {

return Some(sig);

}

}

}

match (runtime_api.validator_set(hash), runtime_api.read_share(hash, id.clone()) {

(Some(validator_set), Some(pok_bytes)) if let Ok(sig) = self.key_store.etf_sign(

&id,

&pok_bytes,

&message, validator_set.len() as u8

) => return Some(sig);

_ => return None;

}

juangirini · 2024-04-22T09:42:10Z

substrate/client/keystore/src/local.rs

+				.map(|pair| pair.acss_recover(pok_bytes, threshold)) {
+				// "IBE.Extract" Q = s*H(message) + DLEQ Proof
+				let extract = etf_pair.sign(&message);
+				// let pk = etf_pair.public();


cleanup

Suggested change

// let pk = etf_pair.public();

juangirini · 2024-04-22T09:46:20Z

substrate/primitives/consensus/beefy/Cargo.toml

+# w3f-bls = { path = "../../../../../bls", features = ["std"] }
+# w3f-bls = { git = "https://github.com/driemworks/bls.git", branch = "etf", features  = ["std"] }


Suggested change

# w3f-bls = { path = "../../../../../bls", features = ["std"] }

# w3f-bls = { git = "https://github.com/driemworks/bls.git", branch = "etf", features = ["std"] }

juangirini · 2024-04-22T09:46:39Z

substrate/primitives/consensus/beefy/src/lib.rs

@@ -200,16 +200,23 @@ pub mod ecdsa_bls_crypto {
 		fn verify(&self, signature: &<Self as RuntimeAppPublic>::Signature, msg: &[u8]) -> bool {
 			// We can not simply call
 			// `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())`
-			// because that invokes ECDSA default verification which performs Blake2b hash
+			// because that invokes ECDSA default verification which perfoms Blake2b hash


Suggested change

// because that invokes ECDSA default verification which perfoms Blake2b hash

// because that invokes ECDSA default verification which performs Blake2b hash

juangirini · 2024-04-22T09:47:04Z

substrate/primitives/consensus/beefy/src/lib.rs

+			// EcdsaBlsPair::verify::<H>(
+			// 	signature.as_inner_ref(),
+			// 	msg,
+			// )
+			// EcdsaBlsPair::verify_with_hasher::<H>(
+			// 	signature.as_inner_ref(),
+			// 	msg,
+			// 	self.as_inner_ref(),
+			// )


Suggested change

// EcdsaBlsPair::verify::<H>(

// signature.as_inner_ref(),

// msg,

// )

// EcdsaBlsPair::verify_with_hasher::<H>(

// signature.as_inner_ref(),

// msg,

// self.as_inner_ref(),

// )

juangirini · 2024-04-22T09:47:23Z

substrate/primitives/consensus/beefy/src/lib.rs

@@ -538,7 +551,8 @@ mod tests {
 	}

 	#[test]
-	#[cfg(feature = "bls-experimental")]
+	// #[cfg(feature = "bls-experimental")]


Suggested change

// #[cfg(feature = "bls-experimental")]

juangirini · 2024-04-22T09:47:44Z

substrate/primitives/core/Cargo.toml

+# etf algs
+# etf-crypto-primitives = { path = "../../../../etf-sdk/etf-crypto-primitives", default-features = false, optional = true }


Suggested change

# etf algs

# etf-crypto-primitives = { path = "../../../../etf-sdk/etf-crypto-primitives", default-features = false, optional = true }

juangirini · 2024-04-22T09:49:00Z

substrate/primitives/core/src/bls.rs

+
+	#[test]
+	fn acss_recover_works() {
+


I guess we need a test here? 😄

juangirini · 2024-04-22T09:49:07Z

substrate/primitives/core/src/bls.rs

@@ -157,7 +182,7 @@ impl<T: BlsBound> TraitPair for Pair<T> {

 	fn derive<Iter: Iterator<Item = DeriveJunction>>(
 		&self,
-		path: Iter,
+		path: Iter, 


Suggested change

path: Iter,

path: Iter,

juangirini · 2024-04-22T09:49:42Z

substrate/primitives/core/src/crypto_bytes.rs

@@ -3,7 +3,7 @@
 // Copyright (C) Parity Technologies (UK) Ltd.
 // SPDX-License-Identifier: Apache-2.0

-// Licensed under the Apache License, Version 2.0 (the "License");
+// Licensed under the Apache License, version 2.0 (the "License");


Suggested change

// Licensed under the Apache License, version 2.0 (the "License");

// Licensed under the Apache License, Version 2.0 (the "License");

juangirini · 2024-04-22T09:50:32Z

substrate/frame/beefy-mmr/src/lib.rs

@@ -76,6 +76,7 @@ where
 pub struct BeefyEcdsaToEthereum;
 impl Convert<sp_consensus_beefy::ecdsa_crypto::AuthorityId, Vec<u8>> for BeefyEcdsaToEthereum {
 	fn convert(beefy_id: sp_consensus_beefy::ecdsa_crypto::AuthorityId) -> Vec<u8> {
+		// Vec::new()


Suggested change

// Vec::new()

juangirini · 2024-04-22T09:50:48Z

substrate/frame/beefy-mmr/src/lib.rs

+		// sp_core::ecdsa::Public::from(beefy_id)
+		// 	.to_eth_address()
+		// 	.map(|v| v.to_vec())
+		// 	.map_err(|_| {
+		// 		log::debug!(target: "runtime::beefy", "Failed to convert BEEFY PublicKey to ETH address!");
+		// 	})
+		// 	.unwrap_or_default()


Suggested change

// sp_core::ecdsa::Public::from(beefy_id)

// .to_eth_address()

// .map(|v| v.to_vec())

// .map_err(|_| {

// log::debug!(target: "runtime::beefy", "Failed to convert BEEFY PublicKey to ETH address!");

// })

// .unwrap_or_default()

juangirini · 2024-04-22T09:54:08Z

substrate/frame/beefy-mmr/src/lib.rs

+pub struct BeefyBlsToEthereum;
+impl Convert<sp_consensus_beefy::bls_crypto::AuthorityId, Vec<u8>> for BeefyBlsToEthereum {
+	fn convert(_beefy_id: sp_consensus_beefy::bls_crypto::AuthorityId) -> Vec<u8> {
+		Vec::new()


Do we need this? We aren't using it as far as I can see

juangirini · 2024-04-22T09:54:40Z

substrate/frame/beefy/src/lib.rs

@@ -55,10 +54,11 @@ mod tests;
 pub use crate::equivocation::{EquivocationOffence, EquivocationReportSystem, TimeSlot};
 pub use pallet::*;

-use crate::equivocation::EquivocationEvidenceFor;
+use crate::equivocation::{EquivocationEvidenceFor};


Suggested change

use crate::equivocation::{EquivocationEvidenceFor};

use crate::equivocation::EquivocationEvidenceFor;

juangirini · 2024-04-22T09:54:50Z

substrate/frame/beefy/src/lib.rs


 const LOG_TARGET: &str = "runtime::beefy";

+


Suggested change

juangirini · 2024-04-22T10:00:00Z

substrate/frame/beefy/src/lib.rs

+		genesis_resharing: &Vec<(T::BeefyId, T::BeefyId, Vec<u8>)>,
+		round_key: Vec<u8>,
+	) -> Result<(), ()>  {
+		// TODO: define 96 as a const


and BoundedVec<u8, ConstU32<1024>> as a type?

juangirini · 2024-04-22T10:01:54Z

substrate/bin/node/runtime/src/lib.rs

+			// let commitments = pallet_beefy::Commitments::<Runtime>::get();
+			// if at as usize >= commitments.len() {
+			// 	return None;
+			// }
+			// Some(commitments[at as usize].clone().into_inner())


Suggested change

// let commitments = pallet_beefy::Commitments::<Runtime>::get();

// if at as usize >= commitments.len() {

// return None;

// }

// Some(commitments[at as usize].clone().into_inner())

juangirini · 2024-04-22T10:05:00Z

substrate/client/consensus/beefy/src/keystore.rs

 #[cfg(feature = "bls-experimental")]
+use sp_core::bls377;
+#[cfg(feature = "bls-experimental")]
 use sp_core::ecdsa_bls377;


Suggested change

#[cfg(feature = "bls-experimental")]

use sp_core::bls377;

#[cfg(feature = "bls-experimental")]

use sp_core::ecdsa_bls377;

#[cfg(feature = "bls-experimental")]

use sp_core::{bls377, ecdsa_bls377};

juangirini · 2024-04-22T10:07:31Z

substrate/client/consensus/beefy/src/lib.rs

@@ -397,6 +401,7 @@ where
 				.ok_or_else(|| Error::Backend("Invalid BEEFY chain".into()))?
 			}

+


Suggested change

juangirini · 2024-04-22T10:11:35Z

substrate/client/consensus/beefy/src/worker.rs

+			.unwrap()
+			.expect("todo");


Would it make sense to return an Error here?

juangirini

It looks good, just some small comments

driemworks · 2024-05-20T14:07:17Z

Outdated.
We decided to not go this route.

driemworks added 10 commits April 1, 2024 16:07

etf finality gadget initial commit: acss integration WIP

6941e20

update bls dep url

1a8e745

update etf path

0ff6a51

seed ACSS on BEEFY genesis

cd06a2d

cleanup, add log messages

607a38f

acss recover and add output to keystore, return pubkey

c2d4449

it works

2796d25

add api

c33317a

add to keystore after derivation

e5a09f3

acss works, can sign votes but finalization not working

0a44c36