feat: add burn to address associations pallet to discourage attacks on storage (#869)

---------

Co-authored-by: Krisztian Pinter <[email protected]>

Author: LGLO

Cargo.lock

@@ -4,6 +4,8 @@ This changelog is based on [Keep A Changelog](https://keepachangelog.com/en/1.1.
 
 # Unreleased
 
+* Added extra constant burn fee in `pallet-address-association` to discourage attacks on pallet storage.
+
 ## Changed
 
 ## Added

changelog.md

@@ -546,10 +546,17 @@ impl pallet_block_production_log::Config for Runtime {
 	type BenchmarkHelper = PalletBlockProductionLogBenchmarkHelper;
 }
 
+parameter_types! {
+	/// Amount of tokens to burn when making irreversible, forever association
+	pub const AddressAssociationBurnAmount: Balance = 1_000_000;
+}
+
 impl pallet_address_associations::Config for Runtime {
 	type WeightInfo = pallet_address_associations::weights::SubstrateWeight<Runtime>;
 
 	type PartnerChainAddress = AccountId;
+	type Currency = Balances;
+	type BurnAmount = AddressAssociationBurnAmount;
 
 	fn genesis_utxo() -> UtxoId {
 		Sidechain::genesis_utxo()

demo/runtime/src/lib.rs

@@ -23,6 +23,7 @@ hex-literal = { workspace = true, optional = true }
 sp-core = { workspace = true, optional = true }
 
 [dev-dependencies]
+pallet-balances = { workspace = true }
 sp-core = { workspace = true }
 sp-io = { workspace = true }
 sp-runtime = { workspace = true }
@@ -34,6 +35,7 @@ std = [
     "frame-support/std",
     "frame-system/std",
     "log/std",
+    "pallet-balances/std",
     "parity-scale-codec/std",
     "scale-info/std",
     "sidechain-domain/std",
@@ -46,5 +48,5 @@ runtime-benchmarks = [
     "frame-system/runtime-benchmarks",
     "sp-runtime/runtime-benchmarks",
     "hex-literal",
-    "sp-core"
+    "sp-core",
 ]

toolkit/address-associations/pallet/Cargo.toml

@@ -15,6 +15,7 @@ use crate::Pallet as AddressAssociations;
 #[benchmarks(where <T as Config>::PartnerChainAddress: Ss58Codec)]
 mod benchmarks {
 	use super::*;
+	use frame_support::traits::{Get, tokens::fungible::Mutate};
 
 	#[benchmark]
 	fn associate_address() {
@@ -31,8 +32,12 @@ mod benchmarks {
 			"1aa8c1b363a207ddadf0c6242a0632f5a557690a327d0245f9d473b983b3d8e1c95a3dd804cab41123c36ddbcb7137b8261c35d5c8ef04ce9d0f8d5c4b3ca607"
 		));
 
+		// Create an account and fund it with sufficient balance
+		let caller: T::AccountId = account("caller", 0, 0);
+		let _ = T::Currency::mint_into(&caller, T::BurnAmount::get() * 2u32.into());
+
 		#[extrinsic_call]
-		_(RawOrigin::None, pc_address, signature, stake_public_key);
+		_(RawOrigin::Signed(caller), pc_address, signature, stake_public_key);
 	}
 
 	impl_benchmark_test_suite!(AddressAssociations, crate::mock::new_test_ext(), crate::mock::Test);

toolkit/address-associations/pallet/src/benchmarking.rs

@@ -1,18 +1,18 @@
-//! Pallet storing associations from main chain public key to parter chain address.
+//! Pallet storing associations from main chain public key to partner chain address.
 //!
 //! ## Purpose of this pallet
 //!
 //! This pallet establishes a many-to-one mapping from Cardano staking keys to Partner Chain addresses.
 //! The purpose of this mapping is primarily to indicate the local PC address to be the recipient of any
 //! block production rewards or cross-chain token transfers credited to a Cardano key holders on a Partner
-//! Chain. Some intended scenarios inlude:
-//! 1. ADA delegators become eligible for block rewards due to their stake poool's operator participating
+//! Chain. Some intended scenarios include:
+//! 1. ADA delegators become eligible for block rewards due to their stake pool's operator participating
 //!    in a Partner Chain network. The on-chain payout mechanism uses data provided by this pallet to
 //!    identify each delegator's Partner Chain address based on their Cardano staking key.
 //! 2. A Partner Chain develops its own cross-chain bridge from Cardano. A Cardano user associates their
 //!    Cardano public key with a Partner Chain address that they control. The user then uses the bridge
 //!    to send some tokens to themselves. The receiving logic in the Partner Chain ledger then uses this
-//!    pallet's data to identify the user's PC account and comlete the transfer.
+//!    pallet's data to identify the user's PC account and complete the transfer.
 //!
 //! ## Usage - PC Builder
 //!
@@ -27,7 +27,7 @@
 //!
 //! ## Usage - PC User
 //!
-//! This pallet expoes a single extrinsic `associate_address` accepting the Cardano public key
+//! This pallet exposes a single extrinsic `associate_address` accepting the Cardano public key
 //! and Partner Chain address to be associated together with a signature confirming that the submitter is
 //! the owner of the associated Cardano public key.
 //!
@@ -89,11 +89,13 @@ pub mod pallet {
 	use super::*;
 	use crate::weights::WeightInfo;
 	use frame_support::pallet_prelude::*;
-	use frame_system::pallet_prelude::OriginFor;
+	use frame_support::traits::fungible::{Inspect, Mutate};
+	use frame_support::traits::tokens::{Fortitude, Precision, Preservation};
+	use frame_system::{ensure_signed, pallet_prelude::OriginFor};
 	use sidechain_domain::{MainchainKeyHash, StakeKeySignature, StakePublicKey, UtxoId};
 
 	/// Current version of the pallet
-	pub const PALLET_VERSION: u32 = 1;
+	pub const PALLET_VERSION: u32 = 2;
 
 	#[pallet::pallet]
 	pub struct Pallet<T>(_);
@@ -107,6 +109,13 @@ pub mod pallet {
 		/// account ID, or some address type specific to the Partner Chain.
 		type PartnerChainAddress: Member + Parameter + MaxEncodedLen;
 
+		/// The currency used for burning tokens when an address association is made
+		type Currency: Mutate<Self::AccountId>;
+
+		/// The amount of tokens to burn when upserting metadata
+		#[pallet::constant]
+		type BurnAmount: Get<<Self::Currency as Inspect<Self::AccountId>>::Balance>;
+
 		/// Function returning the genesis UTXO of the Partner Chain.
 		/// This typically should be wired with the `genesis_utxo` function exposed by `pallet_sidechain`.
 		fn genesis_utxo() -> UtxoId;
@@ -133,6 +142,8 @@ pub mod pallet {
 		MainchainKeyAlreadyAssociated,
 		/// Signals an invalid Cardano key signature
 		InvalidMainchainSignature,
+		/// Could not burn additional fee for occupying space
+		InsufficientBalance,
 	}
 
 	#[pallet::call]
@@ -145,11 +156,12 @@ pub mod pallet {
 		#[pallet::call_index(0)]
 		#[pallet::weight(T::WeightInfo::associate_address())]
 		pub fn associate_address(
-			_origin: OriginFor<T>,
+			origin: OriginFor<T>,
 			partnerchain_address: T::PartnerChainAddress,
 			signature: StakeKeySignature,
 			stake_public_key: StakePublicKey,
 		) -> DispatchResult {
+			let origin_account_id = ensure_signed(origin)?;
 			let genesis_utxo = T::genesis_utxo();
 
 			let stake_key_hash = stake_public_key.hash();
@@ -158,6 +170,14 @@ pub mod pallet {
 				!AddressAssociations::<T>::contains_key(&stake_key_hash),
 				Error::<T>::MainchainKeyAlreadyAssociated
 			);
+			T::Currency::burn_from(
+				&origin_account_id,
+				T::BurnAmount::get(),
+				Preservation::Preserve,
+				Precision::Exact,
+				Fortitude::Force,
+			)
+			.map_err(|_| Error::<T>::InsufficientBalance)?;
 
 			let address_association_message = AddressAssociationSignedMessage {
 				stake_public_key: stake_public_key.clone(),

toolkit/address-associations/pallet/src/lib.rs

@@ -1,11 +1,13 @@
+use frame_support::parameter_types;
 use frame_support::traits::ConstU32;
 use frame_support::{
 	construct_runtime,
 	traits::{ConstU16, ConstU64},
 };
 use hex_literal::hex;
 use sidechain_domain::*;
-use sp_core::H256;
+use sp_core::crypto::Ss58Codec;
+use sp_core::{ConstU128, H256};
 use sp_runtime::{
 	AccountId32, BuildStorage,
 	traits::{BlakeTwo256, IdentityLookup},
@@ -35,7 +37,7 @@ pub mod mock_pallet {
 			partner_chain_address: PartnerChainAddress,
 			main_chain_key_hash: MainchainKeyHash,
 		) {
-			LastNewAssociation::<T>::put((partner_chain_address, main_chain_key_hash))
+			LastNewAssociation::<T>::put((partner_chain_address, main_chain_key_hash));
 		}
 	}
 }
@@ -44,6 +46,7 @@ construct_runtime! {
 	pub enum Test {
 		System: frame_system,
 		AddressAssociations: crate::pallet,
+		Balances: pallet_balances,
 		MockPallet: mock_pallet
 	}
 }
@@ -65,7 +68,7 @@ impl frame_system::Config for Test {
 	type BlockHashCount = ConstU64<250>;
 	type Version = ();
 	type PalletInfo = PalletInfo;
-	type AccountData = ();
+	type AccountData = pallet_balances::AccountData<u128>;
 	type OnNewAccount = ();
 	type OnKilledAccount = ();
 	type SystemWeightInfo = ();
@@ -83,16 +86,58 @@ impl frame_system::Config for Test {
 	type PostTransactions = ();
 }
 
+impl pallet_balances::Config for Test {
+	type MaxLocks = ConstU32<50>;
+	type MaxReserves = ();
+	type ReserveIdentifier = [u8; 8];
+	type Balance = u128;
+	type RuntimeEvent = RuntimeEvent;
+	type DustRemoval = ();
+	type ExistentialDeposit = ConstU128<1>;
+	type AccountStore = System;
+	type WeightInfo = pallet_balances::weights::SubstrateWeight<Test>;
+	type FreezeIdentifier = ();
+	type MaxFreezes = ();
+	type RuntimeHoldReason = RuntimeHoldReason;
+	type RuntimeFreezeReason = ();
+	type DoneSlashHandler = ();
+}
+
+parameter_types! {
+	pub const AssociationFeeBurn: u128 = 1000;
+}
+
+pub(crate) const FUNDED_ACCOUNT: AccountId32 = AccountId32::new([1; 32]);
+
+pub(crate) const STAKE_PUBLIC_KEY: StakePublicKey =
+	StakePublicKey(hex!("2bebcb7fbc74a6e0fd6e00a311698b047b7b659f0e047ff5349dbd984aefc52c"));
+
+pub(crate) fn pc_address() -> AccountId32 {
+	AccountId32::from_ss58check("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY").unwrap()
+}
+
+pub(crate) const VALID_SIGNATURE: [u8; 64] = hex!(
+	"1aa8c1b363a207ddadf0c6242a0632f5a557690a327d0245f9d473b983b3d8e1c95a3dd804cab41123c36ddbcb7137b8261c35d5c8ef04ce9d0f8d5c4b3ca607"
+);
+
 impl crate::pallet::Config for Test {
 	type WeightInfo = ();
 	type PartnerChainAddress = PartnerChainAddress;
 	fn genesis_utxo() -> UtxoId {
 		UtxoId::new(hex!("59104061ffa0d66f9ba0135d6fc6a884a395b10f8ae9cb276fc2c3bfdfedc260"), 1)
 	}
-
+	type Currency = Balances;
+	type BurnAmount = AssociationFeeBurn;
 	type OnNewAssociation = MockPallet;
 }
 
 pub fn new_test_ext() -> sp_io::TestExternalities {
-	frame_system::GenesisConfig::<Test>::default().build_storage().unwrap().into()
+	let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
+	pallet_balances::GenesisConfig::<Test> {
+		balances: vec![(FUNDED_ACCOUNT, 100_000)],
+		dev_accounts: None,
+	}
+	.assimilate_storage(&mut t)
+	.unwrap();
+	t.into()
 }

toolkit/address-associations/pallet/src/mock.rs

@@ -4,40 +4,32 @@ use frame_system::pallet_prelude::OriginFor;
 use hex_literal::hex;
 use mock::*;
 use sidechain_domain::*;
-use sp_core::crypto::Ss58Codec;
 use sp_runtime::AccountId32;
 
 #[test]
 fn saves_new_address_association() {
 	new_test_ext().execute_with(|| {
-			// Alice
-			let stake_public_key = StakePublicKey(hex!(
-				"2bebcb7fbc74a6e0fd6e00a311698b047b7b659f0e047ff5349dbd984aefc52c"
-			));
-			let mc_signature = hex!("1aa8c1b363a207ddadf0c6242a0632f5a557690a327d0245f9d473b983b3d8e1c95a3dd804cab41123c36ddbcb7137b8261c35d5c8ef04ce9d0f8d5c4b3ca607");
+		assert_eq!(mock_pallet::LastNewAssociation::<Test>::get(), None);
+		let initial_balance = Balances::free_balance(&FUNDED_ACCOUNT);
+		assert_ok!(super::Pallet::<Test>::associate_address(
+			OriginFor::<Test>::signed(FUNDED_ACCOUNT),
+			pc_address(),
+			VALID_SIGNATURE.into(),
+			STAKE_PUBLIC_KEY.clone(),
+		));
 
-			// Alice
-			let pc_address =
-					AccountId32::from_ss58check("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY").unwrap();
-
-			assert_eq!(mock_pallet::LastNewAssociation::<Test>::get(), None);
-
-			assert_ok!(
-				super::Pallet::<Test>::associate_address(
-					OriginFor::<Test>::signed(AccountId32::new([1; 32])),
-					pc_address.clone(),
-					mc_signature.into(),
-					stake_public_key.clone(),
-				)
-			);
-
-			assert_eq!(
-				Pallet::<Test>::get_partner_chain_address_for(&stake_public_key),
-				Some(pc_address.clone())
-			);
+		assert_eq!(
+			Pallet::<Test>::get_partner_chain_address_for(&STAKE_PUBLIC_KEY),
+			Some(pc_address())
+		);
 
-			assert_eq!(mock_pallet::LastNewAssociation::<Test>::get(), Some((pc_address,stake_public_key.hash())));
-		})
+		assert_eq!(
+			mock_pallet::LastNewAssociation::<Test>::get(),
+			Some((pc_address(), STAKE_PUBLIC_KEY.hash()))
+		);
+		let final_balance = Balances::free_balance(&FUNDED_ACCOUNT);
+		assert_eq!(final_balance, initial_balance - AssociationFeeBurn::get());
+	})
 }
 
 #[test]
@@ -56,7 +48,7 @@ fn rejects_duplicate_key_association() {
 		);
 		assert_eq!(
 			super::Pallet::<Test>::associate_address(
-				OriginFor::<Test>::signed(AccountId32::new([1; 32])),
+				OriginFor::<Test>::signed(FUNDED_ACCOUNT),
 				AccountId32::new([0; 32]),
 				signature,
 				stake_public_key,
@@ -70,20 +62,34 @@ fn rejects_duplicate_key_association() {
 #[test]
 fn rejects_invalid_mainchain_signature() {
 	new_test_ext().execute_with(|| {
-			let stake_public_key = StakePublicKey(hex!(
-				"fc014cb5f071f5d6a36cb5a7e5f168c86555989445a23d4abec33d280f71aca4"
-			));
-			let signature = StakeKeySignature(hex!("c50828c31d1a61e05fdb943847efd42ce2eadda9c7d21dd2d035e8de66bc56de7f6b1297fba6cb7305f2aac97b5f9168894fb10295c503de6d5fb6ae70bd9a0d"));
+
+			let invalid_signature = StakeKeySignature(hex!("c50828c31d1a61e05fdb943847efd42ce2eadda9c7d21dd2d035e8de66bc56de7f6b1297fba6cb7305f2aac97b5f9168894fb10295c503de6d5fb6ae70bd9a0d"));
 
 			assert_eq!(
 				super::Pallet::<Test>::associate_address(
-					OriginFor::<Test>::signed(AccountId32::new([1; 32])),
-					AccountId32::new([0; 32]),
-					signature,
-					stake_public_key,
+					OriginFor::<Test>::signed(FUNDED_ACCOUNT),
+					pc_address(),
+					invalid_signature,
+					STAKE_PUBLIC_KEY,
 				)
 				.unwrap_err(),
 				Error::<Test>::InvalidMainchainSignature.into()
 			);
 		})
 }
+
+#[test]
+fn rejects_extrinsic_when_origin_account_cannot_pay_extra_fee() {
+	new_test_ext().execute_with(|| {
+		assert_eq!(
+			super::Pallet::<Test>::associate_address(
+				OriginFor::<Test>::signed(AccountId32::new([3; 32])),
+				pc_address(),
+				VALID_SIGNATURE.into(),
+				STAKE_PUBLIC_KEY,
+			)
+			.unwrap_err(),
+			Error::<Test>::InsufficientBalance.into()
+		);
+	})
+}