Merge branch 'master' of github.com:webb-tools/dkg-substrate into fix…

…-handle-signed-refresh-proposal

pallets/bridge-registry/src/lib.rs

@@ -140,7 +140,7 @@ pub mod pallet {
 	#[pallet::storage]
 	#[pallet::getter(fn bridges)]
 	/// Storage for map of all bridges
-	pub(super) type Bridges<T: Config> = StorageMap<
+	pub type Bridges<T: Config> = StorageMap<
 		_,
 		Blake2_256,
 		T::BridgeIndex,
@@ -150,7 +150,7 @@ pub mod pallet {
 	#[pallet::storage]
 	#[pallet::getter(fn resource_to_bridge_index)]
 	/// Mapping of resource to bridge index
-	pub(super) type ResourceToBridgeIndex<T: Config> =
+	pub type ResourceToBridgeIndex<T: Config> =
 		StorageMap<_, Blake2_256, ResourceId, T::BridgeIndex>;
 
 	#[pallet::event]

pallets/dkg-metadata/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "pallet-dkg-metadata"
 description = "FRAME pallet for DKG metadata"
-version = "0.1.0"
+version = "0.2.0"
 authors = { workspace = true }
 license = { workspace = true }
 publish = { workspace = true }
@@ -37,6 +37,7 @@ sp-core = { workspace = true }
 sp-io = { workspace = true }
 sp-staking = { workspace = true }
 sp-keystore = { workspace = true }
+k256 = { version = "0.11.5", default-features = false, features = ["arithmetic", "ecdsa"] }
 
 [features]
 default = ["std"]

pallets/dkg-metadata/src/lib.rs

@@ -144,6 +144,13 @@ use weights::WeightInfo;
 // Reputation assigned to genesis authorities
 pub const INITIAL_REPUTATION: u32 = 1_000_000_000;
 
+// Our goal is to trigger the ShouldExecuteNewKeygen to true if we have passed exactly X blocks from
+// the last session rotation and still have not seen a new key, this value cannot be too small
+// either because we need to accomodate delays between `keygen process starting + completion +
+// posted onchain`. If the delay is smaller than the above then we might inadvertently rotate
+// sessions.
+pub const BLOCKS_TO_WAIT_BEFORE_KEYGEN_RETRY_TRIGGER: u32 = 20;
+
 // Reputation increase awarded to authorities on submission of next public key
 pub const REPUTATION_INCREMENT: u32 = INITIAL_REPUTATION / 1000;
 
@@ -204,6 +211,13 @@ pub mod pallet {
 			+ Into<ecdsa::Public>
 			+ From<ecdsa::Public>
 			+ MaxEncodedLen;
+		/// Convert DKG AuthorityId to a form that would end up in the Merkle Tree.
+		///
+		/// For instance for ECDSA (secp256k1) we want to store uncompressed public keys (65 bytes)
+		/// and later to Ethereum Addresses (160 bits) to simplify using them on Ethereum chain,
+		/// but the rest of the Substrate codebase is storing them compressed (33 bytes) for
+		/// efficiency reasons.
+		type DKGAuthorityToMerkleLeaf: Convert<Self::DKGId, Vec<u8>>;
 		/// Jail lengths for misbehaviours
 		type KeygenJailSentence: Get<BlockNumberFor<Self>>;
 		type SigningJailSentence: Get<BlockNumberFor<Self>>;
@@ -403,12 +417,15 @@ pub mod pallet {
 
 			// Our goal is to trigger the ShouldExecuteNewKeygen if either of the two conditions are
 			// true : 1. A SessionPeriod of blocks have passed from the LastSessionRotationBlock
-			// 2. If 1 is true and we have not yet seen NextKey on chain for the last 10 blocks
-			// check if we have passed exactly `Period` blocks from the last session rotation
+			// 2. If 1 is true and we have not yet seen NextKey on chain for the last
+			// BLOCKS_TO_WAIT_BEFORE_KEYGEN_RETRY_TRIGGER blocks check if we have passed exactly
+			// `Period` blocks from the last session rotation
 			let blocks_passed_since_last_session_rotation =
 				n - LastSessionRotationBlock::<T>::get();
 			if blocks_passed_since_last_session_rotation >= T::SessionPeriod::get() &&
-				blocks_passed_since_last_session_rotation % 10u32.into() == 0u32.into()
+				blocks_passed_since_last_session_rotation %
+					BLOCKS_TO_WAIT_BEFORE_KEYGEN_RETRY_TRIGGER.into() ==
+					0u32.into()
 			{
 				// lets set the ShouldStartDKG to true
 				// we dont set force_keygen to true, that is reserved for emergency rotate
@@ -1358,7 +1375,10 @@ pub mod pallet {
 			// to sign our own key as a means of jumpstarting the mechanism.
 			if let Some(pub_key) = next_pub_key {
 				ShouldSubmitProposerVote::<T>::put(true);
-				let next_nonce = Self::refresh_nonce() + 1u32;
+				// we use the current nonce as the next nonce since the refreshNonce
+				// has been incremented in `change_authorities` incrementing here will lead to a
+				// difference of two
+				let next_nonce = Self::refresh_nonce();
 				let data = Self::create_refresh_proposal(pub_key.1.into(), next_nonce)?;
 				match T::ProposalHandler::handle_unsigned_proposal(data) {
 					Ok(()) => {
@@ -1627,7 +1647,7 @@ impl<T: Config> Pallet<T> {
 		// Hash the external accounts into 32 byte chunks to form the base layer of the merkle tree
 		let mut base_layer: Vec<[u8; 32]> = voters
 			.iter()
-			.map(|account| account.to_raw_vec())
+			.map(|account| T::DKGAuthorityToMerkleLeaf::convert(account.clone()))
 			.map(|account| keccak_256(&account))
 			.collect();
 		// Pad base_layer to have length 2^height

pallets/dkg-metadata/src/mock.rs

@@ -29,8 +29,8 @@ use sp_runtime::{
 	impl_opaque_keys,
 	testing::TestXt,
 	traits::{
-		BlakeTwo256, ConvertInto, Extrinsic as ExtrinsicT, IdentifyAccount, IdentityLookup,
-		OpaqueKeys, Verify,
+		BlakeTwo256, Convert, ConvertInto, Extrinsic as ExtrinsicT, IdentifyAccount,
+		IdentityLookup, OpaqueKeys, Verify,
 	},
 	BuildStorage, Percent, Permill,
 };
@@ -141,6 +141,7 @@ parameter_types! {
 
 impl pallet_dkg_metadata::Config for Test {
 	type DKGId = DKGId;
+	type DKGAuthorityToMerkleLeaf = DKGEcdsaToEthereumAddress;
 	type RuntimeEvent = RuntimeEvent;
 	type OnAuthoritySetChangeHandler = ();
 	type OnDKGPublicKeyChangeHandler = ();
@@ -254,3 +255,23 @@ pub fn new_test_ext_raw_authorities(authorities: Vec<(AccountId, DKGId)>) -> Tes
 	ext.register_extension(KeystoreExt(Arc::new(MemoryKeystore::new()) as KeystorePtr));
 	ext
 }
+
+/// Convert DKG secp256k1 public keys into Ethereum addresses
+pub struct DKGEcdsaToEthereumAddress;
+impl Convert<dkg_runtime_primitives::crypto::AuthorityId, Vec<u8>> for DKGEcdsaToEthereumAddress {
+	fn convert(a: dkg_runtime_primitives::crypto::AuthorityId) -> Vec<u8> {
+		use k256::{ecdsa::VerifyingKey, elliptic_curve::sec1::ToEncodedPoint};
+		let _x = VerifyingKey::from_sec1_bytes(sp_core::crypto::ByteArray::as_slice(&a));
+		VerifyingKey::from_sec1_bytes(sp_core::crypto::ByteArray::as_slice(&a))
+			.map(|pub_key| {
+				// uncompress the key
+				let uncompressed = pub_key.to_encoded_point(false);
+				// convert to ETH address
+				sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].to_vec()
+			})
+			.map_err(|_| {
+				log::error!(target: "runtime::dkg_proposals", "Invalid DKG PublicKey format!");
+			})
+			.unwrap_or_default()
+	}
+}

pallets/dkg-metadata/src/tests.rs

@@ -18,7 +18,7 @@ use crate::{
 	mock::*, AggregatedMisbehaviourReports, AggregatedPublicKeys, Authorities,
 	AuthorityReputations, BestAuthorities, Config, Error, Event, JailedKeygenAuthorities,
 	NextAuthorities, NextBestAuthorities, NextKeygenThreshold, NextSignatureThreshold,
-	INITIAL_REPUTATION, REPUTATION_INCREMENT,
+	RefreshNonce, INITIAL_REPUTATION, REPUTATION_INCREMENT,
 };
 use codec::Encode;
 use dkg_runtime_primitives::{keccak_256, utils::ecdsa, MisbehaviourType, KEY_TYPE};
@@ -875,3 +875,37 @@ fn reputation_is_set_correctly() {
 		}
 	})
 }
+
+#[test]
+fn force_change_authorities_increments_nonce_correctly() {
+	new_test_ext(vec![1, 2, 3, 4, 5]).execute_with(|| {
+		let session_id = 1;
+
+		// prep the next authorities
+		let mut next_authorities: BoundedVec<_, _> = Default::default();
+		let mut next_authorities_raw: Vec<_> = Default::default();
+		for _ in 1..=5 {
+			let authority_id = mock_pub_key();
+			let dkg_id = DKGId::from(authority_id);
+			next_authorities_raw.push(authority_id);
+			next_authorities.try_push(dkg_id).unwrap();
+		}
+
+		let keygen_threshold = 5;
+		let signature_threshold = 3;
+
+		RefreshNonce::<Test>::put(1);
+		let input: BoundedVec<_, _> = mock_pub_key().to_raw_vec().try_into().unwrap();
+		crate::pallet::NextDKGPublicKey::<Test>::put((session_id, input));
+		NextKeygenThreshold::<Test>::put(keygen_threshold);
+		NextSignatureThreshold::<Test>::put(signature_threshold);
+		NextAuthorities::<Test>::put(&next_authorities);
+
+		// force change authorities
+		assert_ok!(DKGMetadata::force_change_authorities(RuntimeOrigin::root(),));
+
+		// the new refresh nonce should only increment by one
+		let expected_refresh_nonce: u32 = 2;
+		assert_eq!(RefreshNonce::<Test>::get(), expected_refresh_nonce);
+	});
+}

pallets/dkg-proposal-handler/src/mock.rs

@@ -287,6 +287,7 @@ parameter_types! {
 impl pallet_dkg_metadata::Config for Test {
 	type DKGId = DKGId;
 	type RuntimeEvent = RuntimeEvent;
+	type DKGAuthorityToMerkleLeaf = DKGEcdsaToEthereumAddress;
 	type OnAuthoritySetChangeHandler = ();
 	type OnDKGPublicKeyChangeHandler = ();
 	type OffChainAuthId = dkg_runtime_primitives::offchain::crypto::OffchainAuthId;

pallets/dkg-proposals/src/mock.rs

@@ -168,6 +168,7 @@ parameter_types! {
 
 impl pallet_dkg_metadata::Config for Test {
 	type DKGId = DKGId;
+	type DKGAuthorityToMerkleLeaf = DKGEcdsaToEthereumAddress;
 	type RuntimeEvent = RuntimeEvent;
 	type OnAuthoritySetChangeHandler = DKGProposals;
 	type OnDKGPublicKeyChangeHandler = ();

standalone/node/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "dkg-standalone-node"
-version = "3.0.0"
+version = "3.1.0"
 build = "build.rs"
 description = { workspace = true }
 authors = { workspace = true }

standalone/runtime/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "dkg-standalone-runtime"
-version = "3.0.0"
+version = "3.1.0"
 description = { workspace = true }
 authors = { workspace = true }
 license = { workspace = true }

standalone/runtime/src/lib.rs

@@ -140,7 +140,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("dkg-standalone-node"),
 	impl_name: create_runtime_str!("dkg-standalone-node"),
 	authoring_version: 1,
-	spec_version: 19,
+	spec_version: 20,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,
@@ -300,7 +300,7 @@ parameter_types! {
 #[cfg(not(feature = "integration-tests"))]
 parameter_types! {
   // How often we trigger a new session.
-  pub const Period: BlockNumber = HOURS;
+  pub const Period: BlockNumber = MINUTES;
   pub const Offset: BlockNumber = 0;
 }
 
@@ -607,6 +607,7 @@ parameter_types! {
 
 impl pallet_dkg_metadata::Config for Runtime {
 	type DKGId = DKGId;
+	type DKGAuthorityToMerkleLeaf = pallet_dkg_proposals::DKGEcdsaToEthereumAddress;
 	type RuntimeEvent = RuntimeEvent;
 	type OnAuthoritySetChangeHandler = DKGProposals;
 	type OnDKGPublicKeyChangeHandler = ();