From 988fde42c5eb56dc496ac3b43cb8eee6a7965f83 Mon Sep 17 00:00:00 2001 From: albert Date: Mon, 9 Dec 2024 20:13:08 +0100 Subject: [PATCH 1/9] chore: skip preflight only ccm --- engine/src/sol/retry_rpc.rs | 23 +++++++++++-------- .../src/state_chain_observer/sc_observer.rs | 2 +- state-chain/cfe-events/src/tests.rs | 3 ++- state-chain/chains/src/sol.rs | 1 + state-chain/chains/src/sol/benchmarking.rs | 1 + state-chain/runtime/src/chainflip.rs | 12 ++++++++-- 6 files changed, 29 insertions(+), 13 deletions(-) diff --git a/engine/src/sol/retry_rpc.rs b/engine/src/sol/retry_rpc.rs index e57f7ee3f1..5db5deb149 100644 --- a/engine/src/sol/retry_rpc.rs +++ b/engine/src/sol/retry_rpc.rs @@ -4,7 +4,7 @@ use crate::{ witness::common::chain_source::{ChainClient, Header}, }; use cf_chains::{ - sol::{SolAddress, SolHash, SolSignature}, + sol::{SolAddress, SolHash, SolSignature, SolanaTransactionData}, Solana, }; use cf_utilities::{make_periodic_tick, task_scope::Scope}; @@ -88,8 +88,10 @@ pub trait SolRetryRpcApi: Clone { config: RpcTransactionConfig, ) -> EncodedConfirmedTransactionWithStatusMeta; - async fn broadcast_transaction(&self, raw_transaction: Vec) - -> anyhow::Result; + async fn broadcast_transaction( + &self, + transaction: SolanaTransactionData, + ) -> anyhow::Result; } #[async_trait::async_trait] @@ -227,10 +229,13 @@ impl SolRetryRpcApi for SolRetryRpcClient { ) .await } - async fn broadcast_transaction(&self, transaction: Vec) -> anyhow::Result { - let encoded_transaction = BASE64_STANDARD.encode(&transaction); + async fn broadcast_transaction( + &self, + transaction: SolanaTransactionData, + ) -> anyhow::Result { + let encoded_transaction = BASE64_STANDARD.encode(&transaction.serialized_transaction); let config = RpcSendTransactionConfig { - skip_preflight: true, + skip_preflight: transaction.skip_preflight, preflight_commitment: None, encoding: Some(UiTransactionEncoding::Base64), max_retries: None, @@ -241,7 +246,7 @@ impl SolRetryRpcApi for SolRetryRpcClient { .request_with_limit( RequestLog::new( "sendTransaction".to_string(), - Some(format!("{:?}, {:?}", transaction, config)), + Some(format!("{:?}, {:?}", transaction.serialized_transaction, config)), ), Box::pin(move |client| { let encoded_transaction = encoded_transaction.clone(); @@ -342,7 +347,7 @@ pub mod mocks { config: RpcTransactionConfig, ) -> EncodedConfirmedTransactionWithStatusMeta; - async fn broadcast_transaction(&self, transaction: Vec) + async fn broadcast_transaction(&self, transaction: SolanaTransactionData) -> anyhow::Result; } } @@ -483,7 +488,7 @@ mod tests { // Checking that encoding and sending the transaction works. let tx_signature = retry_client - .broadcast_transaction(signed_and_serialized_tx).await.unwrap(); + .broadcast_transaction(SolanaTransactionData {serialized_transaction: signed_and_serialized_tx, skip_preflight: true}).await.unwrap(); println!("tx_signature: {:?}", tx_signature); diff --git a/engine/src/state_chain_observer/sc_observer.rs b/engine/src/state_chain_observer/sc_observer.rs index 8ad9b7141a..be7891bb8b 100644 --- a/engine/src/state_chain_observer/sc_observer.rs +++ b/engine/src/state_chain_observer/sc_observer.rs @@ -574,7 +574,7 @@ where let sol_rpc = sol_rpc.clone(); let state_chain_client = state_chain_client.clone(); scope.spawn(async move { - match sol_rpc.broadcast_transaction(payload.serialized_transaction).await { + match sol_rpc.broadcast_transaction(payload).await { Ok(tx_signature) => info!("Solana TransactionBroadcastRequest {broadcast_id:?} success: tx_signature: {tx_signature:?}"), Err(error) => { error!("Error on Solana TransactionBroadcastRequest {broadcast_id:?}: {error:?}"); diff --git a/state-chain/cfe-events/src/tests.rs b/state-chain/cfe-events/src/tests.rs index 84b62d13db..9c09bd6faf 100644 --- a/state-chain/cfe-events/src/tests.rs +++ b/state-chain/cfe-events/src/tests.rs @@ -195,7 +195,8 @@ fn event_decoding() { data: vec![31,41,51,61] }], }, - }).finalize_and_serialize().unwrap() + }).finalize_and_serialize().unwrap(), + skip_preflight: false, } }), "0f01000000010101010101010101010101010101010101010101010101010101010101010139020109090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909020202010a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b01020101041f29333d"); } diff --git a/state-chain/chains/src/sol.rs b/state-chain/chains/src/sol.rs index c68f7193a6..c295df4ace 100644 --- a/state-chain/chains/src/sol.rs +++ b/state-chain/chains/src/sol.rs @@ -57,6 +57,7 @@ pub const MAX_CCM_BYTES_USDC: usize = MAX_TRANSACTION_LENGTH - 740usize; // 492 #[derive(Encode, Decode, TypeInfo, Clone, RuntimeDebug, Default, PartialEq, Eq)] pub struct SolanaTransactionData { pub serialized_transaction: Vec, + pub skip_preflight: bool, } /// A Solana transaction in id is a tuple of the AccountAddress and the slot number. diff --git a/state-chain/chains/src/sol/benchmarking.rs b/state-chain/chains/src/sol/benchmarking.rs index 71bdd3818f..0067a724ea 100644 --- a/state-chain/chains/src/sol/benchmarking.rs +++ b/state-chain/chains/src/sol/benchmarking.rs @@ -44,6 +44,7 @@ impl BenchmarkValue for SolanaTransactionData { serialized_transaction: SolTransaction::benchmark_value() .finalize_and_serialize() .expect("Failed to serialize payload"), + skip_preflight: false, } } } diff --git a/state-chain/runtime/src/chainflip.rs b/state-chain/runtime/src/chainflip.rs index 35e20db33c..7fed6ff306 100644 --- a/state-chain/runtime/src/chainflip.rs +++ b/state-chain/runtime/src/chainflip.rs @@ -70,7 +70,7 @@ use cf_traits::{ ScheduledEgressDetails, }; -use cf_chains::{btc::ScriptPubkey, instances::BitcoinInstance}; +use cf_chains::{btc::ScriptPubkey, instances::BitcoinInstance, sol::api::SolanaTransactionType}; use codec::{Decode, Encode}; use eth::Address as EvmAddress; use frame_support::{ @@ -339,7 +339,15 @@ impl TransactionBuilder> for SolanaTransaction fn build_transaction( signed_call: &SolanaApi, ) -> ::Transaction { - SolanaTransactionData { serialized_transaction: signed_call.chain_encoded() } + SolanaTransactionData { + serialized_transaction: signed_call.chain_encoded(), + skip_preflight: match signed_call.call_type { + // skip_preflight when broadcasting ccm transfers to consume the nonce even if the + // transaction reverts + SolanaTransactionType::CcmTransfer { fallback: _ } => true, + _ => false, + }, + } } fn refresh_unsigned_data(_tx: &mut ::Transaction) { From a3ce5719810bb7a7895f5141a99fd2a9e8b7817f Mon Sep 17 00:00:00 2001 From: albert Date: Mon, 9 Dec 2024 20:29:27 +0100 Subject: [PATCH 2/9] chore: set preflight commitment only with skip_preflight --- engine/src/sol/retry_rpc.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/engine/src/sol/retry_rpc.rs b/engine/src/sol/retry_rpc.rs index 5db5deb149..5a2d2c292f 100644 --- a/engine/src/sol/retry_rpc.rs +++ b/engine/src/sol/retry_rpc.rs @@ -15,7 +15,7 @@ use anyhow::{anyhow, Result}; use base64::{prelude::BASE64_STANDARD, Engine}; use super::{ - commitment_config::CommitmentConfig, + commitment_config::{CommitmentConfig, CommitmentLevel}, rpc::{SolRpcApi, SolRpcClient}, rpc_client_api::*, }; @@ -236,7 +236,13 @@ impl SolRetryRpcApi for SolRetryRpcClient { let encoded_transaction = BASE64_STANDARD.encode(&transaction.serialized_transaction); let config = RpcSendTransactionConfig { skip_preflight: transaction.skip_preflight, - preflight_commitment: None, + // 'Confirmed' for preflight commitment is enough, no need for 'Finalised' + // when broadcasting. + preflight_commitment: if transaction.skip_preflight { + None + } else { + Some(CommitmentLevel::Confirmed) + }, encoding: Some(UiTransactionEncoding::Base64), max_retries: None, min_context_slot: None, From 702b1450fbf2052bd4b560a5f3c9ac7e474cddb1 Mon Sep 17 00:00:00 2001 From: albert Date: Mon, 9 Dec 2024 20:30:27 +0100 Subject: [PATCH 3/9] chore: nit --- engine/src/sol/retry_rpc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/src/sol/retry_rpc.rs b/engine/src/sol/retry_rpc.rs index 5a2d2c292f..1f4e635cf3 100644 --- a/engine/src/sol/retry_rpc.rs +++ b/engine/src/sol/retry_rpc.rs @@ -236,11 +236,11 @@ impl SolRetryRpcApi for SolRetryRpcClient { let encoded_transaction = BASE64_STANDARD.encode(&transaction.serialized_transaction); let config = RpcSendTransactionConfig { skip_preflight: transaction.skip_preflight, - // 'Confirmed' for preflight commitment is enough, no need for 'Finalised' - // when broadcasting. preflight_commitment: if transaction.skip_preflight { None } else { + // 'Confirmed' for preflight commitment is enough, no need for + // 'Finalised' when broadcasting. Some(CommitmentLevel::Confirmed) }, encoding: Some(UiTransactionEncoding::Base64), From f07041cd30c777b639d37bd53f6452fb38fdb6f7 Mon Sep 17 00:00:00 2001 From: albert Date: Mon, 9 Dec 2024 20:49:37 +0100 Subject: [PATCH 4/9] chore: update test --- state-chain/cfe-events/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/state-chain/cfe-events/src/tests.rs b/state-chain/cfe-events/src/tests.rs index 9c09bd6faf..29bc7ef6fa 100644 --- a/state-chain/cfe-events/src/tests.rs +++ b/state-chain/cfe-events/src/tests.rs @@ -198,7 +198,7 @@ fn event_decoding() { }).finalize_and_serialize().unwrap(), skip_preflight: false, } - }), "0f01000000010101010101010101010101010101010101010101010101010101010101010139020109090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909020202010a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b01020101041f29333d"); + }), "0f01000000010101010101010101010101010101010101010101010101010101010101010139020109090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909090909020202010a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b01020101041f29333d00"); } // P2P registration/deregistration From fca5d195928ddc435d6bfb4cb308159b9c0aaa8e Mon Sep 17 00:00:00 2001 From: albert Date: Tue, 10 Dec 2024 08:51:04 +0100 Subject: [PATCH 5/9] chore: add migration --- state-chain/pallets/cf-broadcast/src/lib.rs | 2 +- state-chain/runtime/src/lib.rs | 60 +++++++++++-- state-chain/runtime/src/migrations.rs | 1 + .../solana_transaction_data_migration.rs | 88 +++++++++++++++++++ 4 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 state-chain/runtime/src/migrations/solana_transaction_data_migration.rs diff --git a/state-chain/pallets/cf-broadcast/src/lib.rs b/state-chain/pallets/cf-broadcast/src/lib.rs index 67745cd1cc..e06404dd71 100644 --- a/state-chain/pallets/cf-broadcast/src/lib.rs +++ b/state-chain/pallets/cf-broadcast/src/lib.rs @@ -59,7 +59,7 @@ pub enum PalletConfigUpdate { BroadcastTimeout { blocks: u32 }, } -pub const PALLET_VERSION: StorageVersion = StorageVersion::new(10); +pub const PALLET_VERSION: StorageVersion = StorageVersion::new(11); #[frame_support::pallet] pub mod pallet { diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index bac41474de..609c9fc442 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -18,6 +18,7 @@ use crate::{ }, Offence, }, + migrations::solana_transaction_data_migration::NoopUpgrade, monitoring_apis::{ ActivateKeysBroadcastIds, AuthoritiesInfo, BtcUtxos, EpochState, ExternalChainsBlockHeight, FeeImbalance, FlipSupply, LastRuntimeUpgradeInfo, MonitoringDataV2, OpenDepositChannels, @@ -1293,13 +1294,58 @@ type PalletMigrations = ( pallet_cf_cfe_interface::migrations::PalletMigration, ); -type MigrationsForV1_8 = VersionedMigration< - 2, - 3, - migrations::solana_vault_swaps_migration::SolanaVaultSwapsMigration, - pallet_cf_elections::Pallet, - DbWeight, ->; +type MigrationsForV1_8 = ( + VersionedMigration< + 2, + 3, + migrations::solana_vault_swaps_migration::SolanaVaultSwapsMigration, + pallet_cf_elections::Pallet, + DbWeight, + >, + // Only the Solana Transaction type has changed + VersionedMigration< + 10, + 11, + migrations::solana_transaction_data_migration::SolanaTransactionDataMigration, + pallet_cf_broadcast::Pallet, + DbWeight, + >, + VersionedMigration< + 10, + 11, + NoopUpgrade, + pallet_cf_broadcast::Pallet, + DbWeight, + >, + VersionedMigration< + 10, + 11, + NoopUpgrade, + pallet_cf_broadcast::Pallet, + DbWeight, + >, + VersionedMigration< + 10, + 11, + NoopUpgrade, + pallet_cf_broadcast::Pallet, + DbWeight, + >, + VersionedMigration< + 10, + 11, + NoopUpgrade, + pallet_cf_broadcast::Pallet, + DbWeight, + >, + VersionedMigration< + 10, + 11, + NoopUpgrade, + pallet_cf_broadcast::Pallet, + DbWeight, + >, +); #[cfg(feature = "runtime-benchmarks")] #[macro_use] diff --git a/state-chain/runtime/src/migrations.rs b/state-chain/runtime/src/migrations.rs index b32758552b..e656e3acef 100644 --- a/state-chain/runtime/src/migrations.rs +++ b/state-chain/runtime/src/migrations.rs @@ -2,4 +2,5 @@ pub mod housekeeping; pub mod reap_old_accounts; +pub mod solana_transaction_data_migration; pub mod solana_vault_swaps_migration; diff --git a/state-chain/runtime/src/migrations/solana_transaction_data_migration.rs b/state-chain/runtime/src/migrations/solana_transaction_data_migration.rs new file mode 100644 index 0000000000..86c10530ef --- /dev/null +++ b/state-chain/runtime/src/migrations/solana_transaction_data_migration.rs @@ -0,0 +1,88 @@ +use frame_support::traits::UncheckedOnRuntimeUpgrade; +use pallet_cf_broadcast::BroadcastData; + +use crate::*; +use frame_support::pallet_prelude::Weight; +#[cfg(feature = "try-runtime")] +use sp_runtime::DispatchError; + +use cf_chains::sol::SolanaTransactionData; +use codec::{Decode, Encode}; + +pub mod old { + use cf_chains::sol::{SolMessage, SolSignature}; + use cf_primitives::BroadcastId; + use frame_support::{pallet_prelude::OptionQuery, Twox64Concat}; + + use super::*; + + #[derive(PartialEq, Eq, Encode, Decode)] + pub struct SolanaTransactionData { + pub serialized_transaction: Vec, + } + + #[derive(PartialEq, Eq, Encode, Decode)] + pub struct SolanaBroadcastData { + pub broadcast_id: BroadcastId, + pub transaction_payload: SolanaTransactionData, + pub threshold_signature_payload: SolMessage, + pub transaction_out_id: SolSignature, + pub nominee: Option<::AccountId>, + } + + #[frame_support::storage_alias] + pub type AwaitingBroadcast = + StorageMap; +} + +pub struct SolanaTransactionDataMigration; + +impl UncheckedOnRuntimeUpgrade for SolanaTransactionDataMigration { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + Ok((old::AwaitingBroadcast::iter().count() as u64).encode()) + } + + fn on_runtime_upgrade() -> Weight { + pallet_cf_broadcast::AwaitingBroadcast::::translate_values::< + old::SolanaBroadcastData, + _, + >(|old_sol_broadcast_data| { + Some(BroadcastData:: { + broadcast_id: old_sol_broadcast_data.broadcast_id, + transaction_payload: SolanaTransactionData { + serialized_transaction: old_sol_broadcast_data + .transaction_payload + .serialized_transaction, + skip_preflight: true, + }, + threshold_signature_payload: old_sol_broadcast_data.threshold_signature_payload, + transaction_out_id: old_sol_broadcast_data.transaction_out_id, + nominee: old_sol_broadcast_data.nominee, + }) + }); + + Weight::zero() + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), DispatchError> { + let pre_awaiting_broadcast_count = ::decode(&mut state.as_slice()) + .map_err(|_| DispatchError::from("Failed to decode state"))?; + + let post_awaiting_broadcast_count = + pallet_cf_broadcast::AwaitingBroadcast::::iter().count() + as u64; + + assert_eq!(pre_awaiting_broadcast_count, post_awaiting_broadcast_count); + Ok(()) + } +} + +pub struct NoopUpgrade; + +impl UncheckedOnRuntimeUpgrade for NoopUpgrade { + fn on_runtime_upgrade() -> Weight { + Weight::zero() + } +} From 875c7c0d66ae526d1dd808e3fbfed87dd015d987 Mon Sep 17 00:00:00 2001 From: albert Date: Tue, 10 Dec 2024 08:59:49 +0100 Subject: [PATCH 6/9] chore: run upgrade test --- .github/workflows/ci-development.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-development.yml b/.github/workflows/ci-development.yml index bc66706607..149542ba6e 100644 --- a/.github/workflows/ci-development.yml +++ b/.github/workflows/ci-development.yml @@ -79,7 +79,7 @@ jobs: uses: ./.github/workflows/upgrade-test.yml secrets: inherit with: - run-job: false + run-job: true publish: needs: [package] uses: ./.github/workflows/_30_publish.yml From eec98681eaac1bd6072c0037defed179091dc2f5 Mon Sep 17 00:00:00 2001 From: albert Date: Tue, 10 Dec 2024 10:34:19 +0100 Subject: [PATCH 7/9] chore: restore upgrade flag --- .github/workflows/ci-development.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-development.yml b/.github/workflows/ci-development.yml index 149542ba6e..bc66706607 100644 --- a/.github/workflows/ci-development.yml +++ b/.github/workflows/ci-development.yml @@ -79,7 +79,7 @@ jobs: uses: ./.github/workflows/upgrade-test.yml secrets: inherit with: - run-job: true + run-job: false publish: needs: [package] uses: ./.github/workflows/_30_publish.yml From 0c7eb99963e7a15ae808b3e90d5688e1058c2175 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 10 Dec 2024 14:55:02 +0100 Subject: [PATCH 8/9] chore: use matches! --- state-chain/runtime/src/chainflip.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/state-chain/runtime/src/chainflip.rs b/state-chain/runtime/src/chainflip.rs index 7fed6ff306..05d998309a 100644 --- a/state-chain/runtime/src/chainflip.rs +++ b/state-chain/runtime/src/chainflip.rs @@ -341,12 +341,12 @@ impl TransactionBuilder> for SolanaTransaction ) -> ::Transaction { SolanaTransactionData { serialized_transaction: signed_call.chain_encoded(), - skip_preflight: match signed_call.call_type { - // skip_preflight when broadcasting ccm transfers to consume the nonce even if the - // transaction reverts - SolanaTransactionType::CcmTransfer { fallback: _ } => true, - _ => false, - }, + // skip_preflight when broadcasting ccm transfers to consume the nonce even if the + // transaction reverts + skip_preflight: matches!( + signed_call.call_type, + SolanaTransactionType::CcmTransfer { .. } + ), } } From 48193e6d9aa8bb3d9db3bc61c8814234b24da2f9 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 10 Dec 2024 14:55:14 +0100 Subject: [PATCH 9/9] chore: print sol tx as hex --- engine/src/sol/retry_rpc.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/engine/src/sol/retry_rpc.rs b/engine/src/sol/retry_rpc.rs index 1f4e635cf3..abd172109e 100644 --- a/engine/src/sol/retry_rpc.rs +++ b/engine/src/sol/retry_rpc.rs @@ -252,7 +252,11 @@ impl SolRetryRpcApi for SolRetryRpcClient { .request_with_limit( RequestLog::new( "sendTransaction".to_string(), - Some(format!("{:?}, {:?}", transaction.serialized_transaction, config)), + Some(format!( + "0x{}, {:?}", + hex::encode(&transaction.serialized_transaction), + config + )), ), Box::pin(move |client| { let encoded_transaction = encoded_transaction.clone();