Clean up Events and Errors and add docstrings (#207)

* turn all enclave-bridge events with tuples into structs

* more struct events

* clean up enclave events

* more cleanup and docstrings

enclave-bridge/src/lib.rs

@@ -73,19 +73,46 @@ pub mod pallet {
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
+		/// an indirect invocation has been registered for execution on L2
 		IndirectInvocationRegistered(ShardIdentifier),
-		ShieldFunds(Vec<u8>, BalanceOf<T>),
-		UnshieldedFunds(T::AccountId, BalanceOf<T>),
-		ProcessedParentchainBlock(ShardIdentifier, H256, H256, T::BlockNumber),
-		/// An enclave with [mr_enclave] has published some [hash] with some metadata [data].
+		/// funds have been shielded to L2
+		ShieldFunds {
+			shard: ShardIdentifier,
+			encrypted_beneficiary: Vec<u8>,
+			amount: BalanceOf<T>,
+		},
+		/// funds have been unshielded from L2 back to L1
+		UnshieldedFunds {
+			shard: ShardIdentifier,
+			beneficiary: T::AccountId,
+			amount: BalanceOf<T>,
+		},
+		/// L2 confirmed processing of a parentchain block
+		ProcessedParentchainBlock {
+			shard: ShardIdentifier,
+			block_hash: H256,
+			trusted_calls_merkle_root: H256,
+			block_number: T::BlockNumber,
+		},
+		/// An enclave has published some [hash] with some metadata [data].
 		PublishedHash {
-			fingerprint: EnclaveFingerprint,
+			enclave_fingerprint: EnclaveFingerprint,
 			hash: H256,
 			data: Vec<u8>,
 		},
 		ShardConfigUpdated(ShardIdentifier),
 	}
 
+	#[pallet::error]
+	pub enum Error<T> {
+		/// The shard doesn't match the enclave.
+		WrongFingerprintForShard,
+		/// The number of `extra_topics` passed to `publish_hash` exceeds the limit.
+		TooManyTopics,
+		/// The length of the `data` passed to `publish_hash` exceeds the limit.
+		DataTooLong,
+	}
+
 	#[pallet::storage]
 	#[pallet::getter(fn shard_status)]
 	pub type ShardStatus<T: Config> = StorageMap<
@@ -154,12 +181,12 @@ pub mod pallet {
 				shard,
 				block_hash
 			);
-			Self::deposit_event(Event::ProcessedParentchainBlock(
+			Self::deposit_event(Event::ProcessedParentchainBlock {
 				shard,
 				block_hash,
 				trusted_calls_merkle_root,
 				block_number,
-			));
+			});
 			Ok(().into())
 		}
 
@@ -183,7 +210,11 @@ pub mod pallet {
 				amount,
 				ExistenceRequirement::AllowDeath,
 			)?;
-			Self::deposit_event(Event::ShieldFunds(incognito_account_encrypted, amount));
+			Self::deposit_event(Event::ShieldFunds {
+				shard,
+				encrypted_beneficiary: incognito_account_encrypted,
+				amount,
+			});
 			Ok(().into())
 		}
 
@@ -214,7 +245,7 @@ pub mod pallet {
 					ExistenceRequirement::AllowDeath,
 				)?;
 				<ExecutedUnshieldCalls<T>>::insert(call_hash, 0);
-				Self::deposit_event(Event::UnshieldedFunds(beneficiary, amount));
+				Self::deposit_event(Event::UnshieldedFunds { shard, beneficiary, amount });
 			} else {
 				log::info!(
 					target: ENCLAVE_BRIDGE,
@@ -263,7 +294,7 @@ pub mod pallet {
 
 			Self::deposit_event_indexed(
 				&topics,
-				Event::PublishedHash { fingerprint: enclave.fingerprint(), hash, data },
+				Event::PublishedHash { enclave_fingerprint: enclave.fingerprint(), hash, data },
 			);
 
 			Ok(().into())
@@ -315,16 +346,6 @@ pub mod pallet {
 			Ok(().into())
 		}
 	}
-
-	#[pallet::error]
-	pub enum Error<T> {
-		/// The shard doesn't match the enclave.
-		WrongFingerprintForShard,
-		/// The number of `extra_topics` passed to `publish_hash` exceeds the limit.
-		TooManyTopics,
-		/// The length of the `data` passed to `publish_hash` exceeds the limit.
-		DataTooLong,
-	}
 }
 
 impl<T: Config> Pallet<T> {

enclave-bridge/src/tests/test_indirect_invocation.rs

@@ -69,10 +69,11 @@ fn unshield_is_only_executed_once_for_the_same_call_hash() {
 			call_hash
 		)
 		.is_ok());
-		let expected_event = RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::UnshieldedFunds(
-			beneficiary.clone(),
+		let expected_event = RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::UnshieldedFunds {
+			shard,
+			beneficiary: beneficiary.clone(),
 			amount,
-		));
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 
 		System::reset_events();
@@ -117,10 +118,11 @@ fn verify_unshield_funds_works() {
 
 		assert_eq!(Balances::free_balance(bonding_account.clone()), 100);
 
-		let expected_event = RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::ShieldFunds(
-			incognito_account_encrypted,
-			100,
-		));
+		let expected_event = RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::ShieldFunds {
+			shard,
+			encrypted_beneficiary: incognito_account_encrypted,
+			amount: 100,
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 
 		assert!(EnclaveBridge::unshield_funds(
@@ -133,8 +135,11 @@ fn verify_unshield_funds_works() {
 		.is_ok());
 		assert_eq!(Balances::free_balance(bonding_account), 50);
 
-		let expected_event =
-			RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::UnshieldedFunds(beneficiary, 50));
+		let expected_event = RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::UnshieldedFunds {
+			shard,
+			beneficiary,
+			amount: 50,
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 	})
 }
@@ -230,12 +235,12 @@ fn confirm_processed_parentchain_block_works() {
 		));
 
 		let expected_event =
-			RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::ProcessedParentchainBlock(
-				ShardIdentifier::default(),
+			RuntimeEvent::EnclaveBridge(EnclaveBridgeEvent::ProcessedParentchainBlock {
+				shard: ShardIdentifier::default(),
 				block_hash,
-				merkle_root,
+				trusted_calls_merkle_root: merkle_root,
 				block_number,
-			));
+			});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 	})
 }

enclave-bridge/src/tests/test_publish_hash.rs

@@ -65,7 +65,7 @@ fn publish_hash_works() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: EnclaveBridgeEvent::PublishedHash {
-						fingerprint: fingerprint.into(),
+						enclave_fingerprint: fingerprint.into(),
 						hash,
 						data
 					}
@@ -75,7 +75,7 @@ fn publish_hash_works() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: EnclaveBridgeEvent::PublishedHash {
-						fingerprint: fingerprint.into(),
+						enclave_fingerprint: fingerprint.into(),
 						hash,
 						data: vec![]
 					}

parentchain/src/lib.rs

@@ -21,6 +21,7 @@ pub mod pallet {
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
+		/// a parentchain block has been registered
 		SetBlock { block_number: T::BlockNumber, parent_hash: T::Hash, block_hash: T::Hash },
 	}
 

sidechain/src/lib.rs

@@ -57,8 +57,20 @@ pub mod pallet {
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
-		ProposedSidechainBlock(T::AccountId, H256),
-		FinalizedSidechainBlock(T::AccountId, H256),
+		/// a sidechain block has been finalized
+		FinalizedSidechainBlock {
+			shard: ShardIdentifier,
+			block_header_hash: H256,
+			validateer: T::AccountId,
+		},
+	}
+
+	#[pallet::error]
+	pub enum Error<T> {
+		/// A proposed block is unexpected.
+		ReceivedUnexpectedSidechainBlock,
+		/// The value for the next finalization candidate is invalid.
+		InvalidNextFinalizationCandidateBlockNumber,
 	}
 
 	#[pallet::storage]
@@ -123,14 +135,6 @@ pub mod pallet {
 			Ok(().into())
 		}
 	}
-
-	#[pallet::error]
-	pub enum Error<T> {
-		/// A proposed block is unexpected.
-		ReceivedUnexpectedSidechainBlock,
-		/// The value for the next finalization candidate is invalid.
-		InvalidNextFinalizationCandidateBlockNumber,
-	}
 }
 
 impl<T: Config> Pallet<T> {
@@ -146,7 +150,11 @@ impl<T: Config> Pallet<T> {
 			shard,
 			block_header_hash
 		);
-		Self::deposit_event(Event::FinalizedSidechainBlock(sender.clone(), block_header_hash));
+		Self::deposit_event(Event::FinalizedSidechainBlock {
+			shard,
+			block_header_hash,
+			validateer: sender.clone(),
+		});
 	}
 }
 

sidechain/src/tests.rs

@@ -79,8 +79,11 @@ fn confirm_imported_sidechain_block_works_for_correct_shard() {
 			hash
 		));
 
-		let expected_event =
-			RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(signer7, hash));
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard: shard7,
+			block_header_hash: hash,
+			validateer: signer7,
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 	})
 }
@@ -201,10 +204,11 @@ fn dont_process_confirmation_of_second_registered_enclave() {
 
 		System::reset_events();
 		assert_ok!(confirm_sidechain_block(shard7, TEST6_SIGNER_PUB, 1, 2, H256::default(), false));
-		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(
-			get_signer(TEST6_SIGNER_PUB),
-			H256::default(),
-		));
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard: shard7,
+			block_header_hash: H256::default(),
+			validateer: get_signer(TEST6_SIGNER_PUB),
+		});
 		assert!(!System::events().iter().any(|a| a.event == expected_event));
 	})
 }
@@ -235,10 +239,11 @@ fn confirm_imported_sidechain_block_works_for_correct_shard_with_updated_fingerp
 			hash
 		));
 
-		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(
-			enclave_signer.clone(),
-			hash,
-		));
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard,
+			block_header_hash: hash,
+			validateer: enclave_signer.clone(),
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 
 		let new_fingerprint = EnclaveFingerprint::from([2u8; 32]);
@@ -259,10 +264,11 @@ fn confirm_imported_sidechain_block_works_for_correct_shard_with_updated_fingerp
 			hash
 		));
 
-		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(
-			enclave_signer.clone(),
-			hash,
-		));
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard,
+			block_header_hash: hash,
+			validateer: enclave_signer.clone(),
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 
 		run_to_block(2);
@@ -279,8 +285,11 @@ fn confirm_imported_sidechain_block_works_for_correct_shard_with_updated_fingerp
 			hash
 		));
 
-		let expected_event =
-			RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(enclave_signer, hash));
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard,
+			block_header_hash: hash,
+			validateer: enclave_signer,
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 	})
 }
@@ -303,10 +312,11 @@ fn two_sidechains_with_different_fingerprint_works() {
 			hash1
 		));
 
-		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(
-			enclave_signer1.clone(),
-			hash1,
-		));
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard: shard1,
+			block_header_hash: hash1,
+			validateer: enclave_signer1.clone(),
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 
 		run_to_block(2);
@@ -325,10 +335,11 @@ fn two_sidechains_with_different_fingerprint_works() {
 			hash2
 		));
 
-		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(
-			enclave_signer2.clone(),
-			hash2,
-		));
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard: shard2,
+			block_header_hash: hash2,
+			validateer: enclave_signer2.clone(),
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 
 		let shard_status1 = EnclaveBridge::shard_status(shard1).unwrap();
@@ -411,10 +422,11 @@ fn confirm_sidechain_block(
 	)?;
 
 	if assert_event {
-		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock(
-			signer,
+		let expected_event = RuntimeEvent::Sidechain(SidechainEvent::FinalizedSidechainBlock {
+			shard,
 			block_header_hash,
-		));
+			validateer: signer,
+		});
 		assert!(System::events().iter().any(|a| a.event == expected_event));
 	}
 	Ok(().into())