feat: solana resigning

state-chain/cf-integration-tests/src/solana.rs

@@ -1,27 +1,35 @@
 #![cfg(test)]
 
+use std::marker::PhantomData;
+
 use super::*;
 use cf_chains::{
 	address::{AddressConverter, AddressDerivationApi, EncodedAddress},
 	assets::any::Asset,
 	ccm_checker::CcmValidityError,
 	sol::{
-		api::{SolanaEnvironment, SolanaTransactionBuildingError},
+		api::{SolanaApi, SolanaEnvironment, SolanaTransactionBuildingError},
 		sol_tx_core::sol_test_values,
+		transaction_builder::SolanaTransactionBuilder,
 		SolAddress, SolApiEnvironment, SolCcmAccounts, SolCcmAddress, SolHash, SolPubkey,
 		SolTrackedData, SolanaCrypto,
 	},
 	CcmChannelMetadata, CcmDepositMetadata, Chain, ChainState, ExecutexSwapAndCallError,
-	ForeignChainAddress, SetAggKeyWithAggKey, SetAggKeyWithAggKeyError, Solana, SwapOrigin,
+	ForeignChainAddress, RequiresSignatureRefresh, SetAggKeyWithAggKey, SetAggKeyWithAggKeyError,
+	Solana, SwapOrigin, TransactionBuilder,
 };
 use cf_primitives::{AccountRole, AuthorityCount, ForeignChain, SwapId};
 use cf_test_utilities::{assert_events_match, assert_has_matching_event};
+use cf_utilities::bs58_array;
 use codec::Encode;
 use frame_support::traits::{OnFinalize, UnfilteredDispatchable};
 use pallet_cf_ingress_egress::DepositWitness;
 use pallet_cf_validator::RotationPhase;
 use state_chain_runtime::{
-	chainflip::{address_derivation::AddressDerivation, ChainAddressConverter, SolEnvironment},
+	chainflip::{
+		address_derivation::AddressDerivation, ChainAddressConverter, SolEnvironment,
+		SolanaTransactionBuilder as RuntimeSolanaTransactionBuilder,
+	},
 	Runtime, RuntimeCall, RuntimeEvent, SolanaIngressEgress, SolanaInstance, SolanaThresholdSigner,
 	Swapping,
 };
@@ -522,3 +530,81 @@ fn failed_ccm_does_not_consume_durable_nonce() {
 			)
 		});
 }
+
+#[test]
+fn solana_resigning() {
+	use crate::solana::sol_test_values::TEST_DURABLE_NONCE;
+
+	const EPOCH_BLOCKS: u32 = 100;
+	const MAX_AUTHORITIES: AuthorityCount = 10;
+	super::genesis::with_test_defaults()
+		.blocks_per_epoch(EPOCH_BLOCKS)
+		.max_authorities(MAX_AUTHORITIES)
+		.with_additional_accounts(&[
+			(DORIS, AccountRole::LiquidityProvider, 5 * FLIPPERINOS_PER_FLIP),
+			(ZION, AccountRole::Broker, 5 * FLIPPERINOS_PER_FLIP),
+			(ALICE, AccountRole::Broker, 5 * FLIPPERINOS_PER_FLIP),
+			(BOB, AccountRole::Broker, 5 * FLIPPERINOS_PER_FLIP),
+		])
+		.build()
+		.execute_with(|| {
+			let (mut testnet, _, _) = network::fund_authorities_and_join_auction(MAX_AUTHORITIES);
+			assert_ok!(RuntimeCall::SolanaVault(
+				pallet_cf_vaults::Call::<Runtime, SolanaInstance>::initialize_chain {}
+			)
+			.dispatch_bypass_filter(pallet_cf_governance::RawOrigin::GovernanceApproval.into()));
+			testnet.move_to_the_next_epoch();
+
+			setup_sol_environments();
+			register_refund_addresses(&DORIS);
+			setup_pool_and_accounts(vec![Asset::Sol, Asset::SolUsdc], OrderType::LimitOrder);
+
+			const CURRENT_SIGNER: [u8; 32] = [3u8; 32];
+
+			let mut transaction = SolanaTransactionBuilder::transfer_native(
+				10000000,
+				SolAddress(bs58_array("EwVmJgZwHBzmdVUzdujfbxFdaG25PMzbPLx8F7PvhWgs")),
+				CURRENT_SIGNER.into(),
+				(SolAddress(bs58_array("2cNMwUCF51djw2xAiiU54wz1WrU8uG4Q8Kp8nfEuwghw")), TEST_DURABLE_NONCE),
+				100,
+			).unwrap();
+			transaction.signatures = vec![[1u8; 64].into()];
+
+			let original_account_keys = transaction.message.account_keys.clone();
+
+			let apicall = SolanaApi {
+				call_type: cf_chains::sol::api::SolanaTransactionType::Transfer,
+				transaction,
+				signer: Some(CURRENT_SIGNER.into()),
+				_phantom: PhantomData::<SolEnvironment>,
+			};
+
+			let modified_call = RuntimeSolanaTransactionBuilder::requires_signature_refresh(
+				&apicall,
+				&Default::default(),
+				Some([5u8; 32].into()),
+			);
+			if let RequiresSignatureRefresh::True(call) = modified_call {
+				let agg_key = <SolEnvironment as SolanaEnvironment>::current_agg_key().unwrap();
+				let transaction = call.clone().unwrap().transaction;
+				for (modified_key, original_key) in transaction.message.account_keys.iter().zip(original_account_keys.iter()) {
+					if *original_key != SolPubkey::from(CURRENT_SIGNER) {
+						assert_eq!(modified_key, original_key);
+						assert_ne!(*modified_key, SolPubkey::from(agg_key));
+					} else {
+						assert_eq!(*modified_key, SolPubkey::from(agg_key));
+					}
+				}
+				let serialized_tx = transaction
+					.clone()
+					.finalize_and_serialize().unwrap();
+
+				// Compare against a manually crafted transaction that works with the current test values and
+				// agg_key. Not comparing the first byte (number of signatures) nor the signature itself
+				let expected_serialized_tx = hex_literal::hex!("010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000306f68d61e8d834034cf583f486f2a08ef53ce4134ed41c4d88f4720c39518745b617eb2b10d3377bda2bc7bea65bec6b8372f4fc3463ec2cd6f9fde4b2c633d192cf1dd130e0341d60a0771ac40ea7900106a423354d2ecd6e609bd5e2ed833dec00000000000000000000000000000000000000000000000000000000000000000306466fe5211732ffecadba72c39be7bc8ce5bbc5f7126b2c439b3a4000000006a7d517192c568ee08a845f73d29788cf035c3145b21ab344d8062ea9400000c27e9074fac5e8d36cf04f94a0606fdd8ddbb420e99a489c7915ce5699e4890004030301050004040000000400090364000000000000000400050284030000030200020c020000008096980000000000").to_vec();
+				assert_eq!(&serialized_tx[1+64..], &expected_serialized_tx[1+64..]);
+			} else {
+				panic!("RequiresSignatureRefresh is False");
+			}
+		});
+}

state-chain/chains/src/lib.rs

@@ -4,6 +4,7 @@
 #![feature(split_array)]
 
 use core::{fmt::Display, iter::Step};
+use sp_std::marker::PhantomData;
 
 use crate::{
 	benchmarking_value::{BenchmarkValue, BenchmarkValueExtended},
@@ -23,7 +24,8 @@ use frame_support::{
 		traits::{AtLeast32BitUnsigned, CheckedAdd, CheckedSub},
 		BoundedVec, DispatchError,
 	},
-	Blake2_256, CloneNoBound, DebugNoBound, EqNoBound, Parameter, PartialEqNoBound, StorageHasher,
+	Blake2_256, CloneNoBound, DebugNoBound, EqNoBound, Never, Parameter, PartialEqNoBound,
+	StorageHasher,
 };
 use instances::{ChainCryptoInstanceAlias, ChainInstanceAlias};
 use scale_info::TypeInfo;
@@ -390,7 +392,7 @@ where
 		call: &Call,
 		payload: &<<C as Chain>::ChainCrypto as ChainCrypto>::Payload,
 		maybe_current_onchain_key: Option<<<C as Chain>::ChainCrypto as ChainCrypto>::AggKey>,
-	) -> bool;
+	) -> RequiresSignatureRefresh<C::ChainCrypto, Call>;
 
 	/// Calculate the Units of gas that is allowed to make this call.
 	fn calculate_gas_limit(_call: &Call) -> Option<U256> {
@@ -703,3 +705,9 @@ pub struct ChannelRefundParameters {
 	pub refund_address: ForeignChainAddress,
 	pub min_price: Price,
 }
+
+pub enum RequiresSignatureRefresh<C: ChainCrypto, Api: ApiCall<C>> {
+	True(Option<Api>),
+	False,
+	_Phantom(PhantomData<C>, Never),
+}

state-chain/chains/src/mocks.rs

@@ -428,7 +428,11 @@ impl<C: Chain<Transaction = MockTransaction>, Call: ApiCall<C::ChainCrypto>>
 		_call: &Call,
 		_payload: &<<C as Chain>::ChainCrypto as ChainCrypto>::Payload,
 		_maybe_current_on_chain_key: Option<<<C as Chain>::ChainCrypto as ChainCrypto>::AggKey>,
-	) -> bool {
-		REQUIRES_REFRESH.with(|is_valid| *is_valid.borrow())
+	) -> RequiresSignatureRefresh<C::ChainCrypto, Call> {
+		if REQUIRES_REFRESH.with(|is_valid| *is_valid.borrow()) {
+			RequiresSignatureRefresh::True(None)
+		} else {
+			RequiresSignatureRefresh::False
+		}
 	}
 }

state-chain/chains/src/sol/api.rs

@@ -131,9 +131,10 @@ pub enum SolanaTransactionType {
 pub struct SolanaApi<Environment: 'static> {
 	pub call_type: SolanaTransactionType,
 	pub transaction: SolTransaction,
+	pub signer: Option<SolAddress>,
 	#[doc(hidden)]
 	#[codec(skip)]
-	_phantom: PhantomData<Environment>,
+	pub _phantom: PhantomData<Environment>,
 }
 
 impl<Environment: SolanaEnvironment> SolanaApi<Environment> {
@@ -158,6 +159,7 @@ impl<Environment: SolanaEnvironment> SolanaApi<Environment> {
 		Ok(Self {
 			call_type: SolanaTransactionType::BatchFetch,
 			transaction,
+			signer: None,
 			_phantom: Default::default(),
 		})
 	}
@@ -210,6 +212,7 @@ impl<Environment: SolanaEnvironment> SolanaApi<Environment> {
 					Self {
 						call_type: SolanaTransactionType::Transfer,
 						transaction,
+						signer: None,
 						_phantom: Default::default(),
 					},
 					vec![egress_id],
@@ -255,6 +258,7 @@ impl<Environment: SolanaEnvironment> SolanaApi<Environment> {
 		Ok(Self {
 			call_type: SolanaTransactionType::RotateAggKey,
 			transaction,
+			signer: None,
 			_phantom: Default::default(),
 		})
 	}
@@ -371,6 +375,7 @@ impl<Environment: SolanaEnvironment> SolanaApi<Environment> {
 		Ok(Self {
 			call_type: SolanaTransactionType::CcmTransfer,
 			transaction,
+			signer: None,
 			_phantom: Default::default(),
 		})
 	}
@@ -384,9 +389,10 @@ impl<Env: 'static> ApiCall<SolanaCrypto> for SolanaApi<Env> {
 	fn signed(
 		mut self,
 		threshold_signature: &<SolanaCrypto as ChainCrypto>::ThresholdSignature,
-		_signer: <SolanaCrypto as ChainCrypto>::AggKey,
+		signer: <SolanaCrypto as ChainCrypto>::AggKey,
 	) -> Self {
 		self.transaction.signatures = vec![*threshold_signature];
+		self.signer = Some(signer);
 		self
 	}
 
@@ -403,11 +409,11 @@ impl<Env: 'static> ApiCall<SolanaCrypto> for SolanaApi<Env> {
 	}
 
 	fn refresh_replay_protection(&mut self) {
-		todo!()
+		// No replay protection refresh for Solana.
 	}
 
 	fn signer(&self) -> Option<<SolanaCrypto as ChainCrypto>::AggKey> {
-		todo!()
+		self.signer
 	}
 }
 
@@ -510,6 +516,7 @@ impl<Environment: SolanaEnvironment> SetGovKeyWithAggKey<SolanaCrypto> for Solan
 		Ok(Self {
 			call_type: SolanaTransactionType::SetGovKeyWithAggKey,
 			transaction,
+			signer: None,
 			_phantom: Default::default(),
 		})
 	}

state-chain/pallets/cf-broadcast/src/lib.rs

@@ -15,7 +15,7 @@ use cf_primitives::{BroadcastId, ThresholdSignatureRequestId};
 
 use cf_chains::{
 	address::IntoForeignChainAddress, ApiCall, Chain, ChainCrypto, FeeRefundCalculator,
-	RetryPolicy, TransactionBuilder, TransactionMetadata as _,
+	RequiresSignatureRefresh, RetryPolicy, TransactionBuilder, TransactionMetadata as _,
 };
 use cf_traits::{
 	impl_pallet_safe_mode, offence_reporting::OffenceReporter, BroadcastNomination, Broadcaster,
@@ -773,13 +773,18 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		if let Some(broadcast_data) = AwaitingBroadcast::<T, I>::get(broadcast_id) {
 			// If the broadcast is not pending, we should not retry.
-			if let Some(api_call) = PendingApiCalls::<T, I>::get(broadcast_id) {
-				if T::TransactionBuilder::requires_signature_refresh(
-					&api_call,
-					&broadcast_data.threshold_signature_payload,
-					CurrentOnChainKey::<T, I>::get(),
-				) {
+			if let Some(mut api_call) = PendingApiCalls::<T, I>::get(broadcast_id) {
+				if let RequiresSignatureRefresh::True(maybe_modified_apicall) =
+					T::TransactionBuilder::requires_signature_refresh(
+						&api_call,
+						&broadcast_data.threshold_signature_payload,
+						CurrentOnChainKey::<T, I>::get(),
+					) {
 					Self::deposit_event(Event::<T, I>::ThresholdSignatureInvalid { broadcast_id });
+					if let Some(modified_apicall) = maybe_modified_apicall {
+						PendingApiCalls::<T, I>::insert(broadcast_id, modified_apicall.clone());
+						api_call = modified_apicall;
+					}
 					Self::threshold_sign(api_call, broadcast_id, true);
 					log::info!(
 						"Signature is invalid -> rescheduled threshold signature for broadcast id {}.",

state-chain/runtime/src/chainflip.rs

@@ -54,8 +54,8 @@ use cf_chains::{
 	},
 	AnyChain, ApiCall, Arbitrum, CcmChannelMetadata, CcmDepositMetadata, Chain, ChainCrypto,
 	ChainEnvironment, ChainState, ChannelRefundParameters, DepositChannel, ForeignChain,
-	ReplayProtectionProvider, SetCommKeyWithAggKey, SetGovKeyWithAggKey, Solana,
-	TransactionBuilder,
+	ReplayProtectionProvider, RequiresSignatureRefresh, SetCommKeyWithAggKey, SetGovKeyWithAggKey,
+	Solana, TransactionBuilder,
 };
 use cf_primitives::{chains::assets, AccountRole, Asset, BasisPoints, Beneficiaries, ChannelId};
 use cf_traits::{
@@ -234,8 +234,14 @@ macro_rules! impl_transaction_builder_for_evm_chain {
 				call: &$chain_api<$env>,
 				_payload: &<EvmCrypto as ChainCrypto>::Payload,
 				maybe_current_on_chain_key: Option<<EvmCrypto as ChainCrypto>::AggKey>
-			) -> bool {
-				maybe_current_on_chain_key.map_or(false, |current_on_chain_key| call.signer().is_some_and(|signer|current_on_chain_key != signer ))
+			) -> RequiresSignatureRefresh<EvmCrypto, $chain_api<$env>> {
+				maybe_current_on_chain_key.map_or(RequiresSignatureRefresh::False,
+					|current_on_chain_key| if call.signer().is_some_and(|signer|current_on_chain_key != signer ) {
+						RequiresSignatureRefresh::True(None)
+					} else {
+						RequiresSignatureRefresh::False
+					}
+				)
 			}
 
 			/// Calculate the gas limit for a this evm chain's call, using the current gas price.
@@ -283,10 +289,14 @@ impl TransactionBuilder<Polkadot, PolkadotApi<DotEnvironment>> for DotTransactio
 		call: &PolkadotApi<DotEnvironment>,
 		payload: &<<Polkadot as Chain>::ChainCrypto as ChainCrypto>::Payload,
 		_maybe_current_onchain_key: Option<<PolkadotCrypto as ChainCrypto>::AggKey>,
-	) -> bool {
+	) -> RequiresSignatureRefresh<PolkadotCrypto, PolkadotApi<DotEnvironment>> {
 		// Current key and signature are irrelevant. The only thing that can invalidate a polkadot
 		// transaction is if the payload changes due to a runtime version update.
-		&call.threshold_signature_payload() != payload
+		if &call.threshold_signature_payload() != payload {
+			RequiresSignatureRefresh::True(None)
+		} else {
+			RequiresSignatureRefresh::False
+		}
 	}
 }
 
@@ -308,14 +318,14 @@ impl TransactionBuilder<Bitcoin, BitcoinApi<BtcEnvironment>> for BtcTransactionB
 		_call: &BitcoinApi<BtcEnvironment>,
 		_payload: &<<Bitcoin as Chain>::ChainCrypto as ChainCrypto>::Payload,
 		_maybe_current_onchain_key: Option<<BitcoinCrypto as ChainCrypto>::AggKey>,
-	) -> bool {
+	) -> RequiresSignatureRefresh<BitcoinCrypto, BitcoinApi<BtcEnvironment>> {
 		// The payload for a Bitcoin transaction will never change and so it doesnt need to be
 		// checked here. We also dont need to check for the signature here because even if we are in
 		// the next epoch and the key has changed, the old signature for the btc tx is still valid
 		// since its based on those old input UTXOs. In fact, we never have to resign btc txs and
 		// the btc tx is always valid as long as the input UTXOs are valid. Therefore, we don't have
 		// to check anything here and just rebroadcast.
-		false
+		RequiresSignatureRefresh::False
 	}
 }
 
@@ -341,16 +351,31 @@ impl TransactionBuilder<Solana, SolanaApi<SolEnvironment>> for SolanaTransaction
 	}
 
 	fn requires_signature_refresh(
-		_call: &SolanaApi<SolEnvironment>,
+		call: &SolanaApi<SolEnvironment>,
 		_payload: &<<Solana as Chain>::ChainCrypto as ChainCrypto>::Payload,
-		_maybe_current_onchain_key: Option<<SolanaCrypto as ChainCrypto>::AggKey>,
-	) -> bool {
-		// We use Durable Nonce mechanism to avoid the 150 blocks expiry period and so
-		// transactions won't expire. Then, the only reason to resign would be if the
-		// payload has been updated or the aggKey has been updated (key rotation).
-		// The payload won't be refreshed, as explained above, and the the broadcast
-		// barrier prevents a transaction from requiring to be resigned by a new aggKey.
-		false
+		maybe_current_onchain_key: Option<<SolanaCrypto as ChainCrypto>::AggKey>,
+	) -> RequiresSignatureRefresh<SolanaCrypto, SolanaApi<SolEnvironment>> {
+		// The only reason to resign would be if the aggKey has been updated on chain (key rotation)
+		// and this apicall is signed with the old key and is still pending. In this case, we need
+		// to modify the apicall, by replacing the aggkey with the new key, in the key_accounts in
+		// the tx's message to create a new valid threshold signing payload.
+		maybe_current_onchain_key.map_or(RequiresSignatureRefresh::False, |current_on_chain_key| {
+			match call.signer() {
+				Some(signer) if signer != current_on_chain_key => {
+					let mut modified_call = (*call).clone();
+					// the unwrap should be safe because we are in the code where on chain key
+					// already exists (see above) and so the active_epoch_key() should also exist.
+					let current_aggkey = SolanaThresholdSigner::active_epoch_key().unwrap().key;
+					for key in modified_call.transaction.message.account_keys.iter_mut() {
+						if *key == signer.into() {
+							*key = current_aggkey.into()
+						}
+					}
+					RequiresSignatureRefresh::True(Some(modified_call.clone()))
+				},
+				_ => RequiresSignatureRefresh::False,
+			}
+		})
 	}
 }