From ad57d2fc8a690fde82135d98674a6a8890f6274f Mon Sep 17 00:00:00 2001 From: "Timothy N. Tsvetkov" Date: Fri, 15 Nov 2024 15:41:18 +0000 Subject: [PATCH] feat(executor): recreate holder when size doesn't match --- executor/src/lib.rs | 2 +- executor/src/transactions.rs | 48 +++++++++++++------- executor/src/transactions/holder.rs | 41 ++++++++--------- executor/src/transactions/ongoing.rs | 30 ++++++++++-- executor/src/transactions/preflight_error.rs | 8 +++- 5 files changed, 85 insertions(+), 44 deletions(-) diff --git a/executor/src/lib.rs b/executor/src/lib.rs index 9d8de52..357f5d5 100644 --- a/executor/src/lib.rs +++ b/executor/src/lib.rs @@ -333,7 +333,7 @@ impl Executor { if current_len > signatures.capacity() { signatures.reserve(current_len - signatures.capacity()); } - for tx in self.pending_transactions.iter() { + for tx in &self.pending_transactions { signatures.push(*tx.key()); } diff --git a/executor/src/transactions.rs b/executor/src/transactions.rs index f70a22e..51591c6 100644 --- a/executor/src/transactions.rs +++ b/executor/src/transactions.rs @@ -97,11 +97,7 @@ pub struct TransactionBuilder { /// ## Utility methods. impl TransactionBuilder { - pub async fn new( - solana_api: SolanaApi, - neon_api: NeonApi, - config: Config, - ) -> anyhow::Result { + pub async fn new(solana_api: SolanaApi, neon_api: NeonApi, config: Config) -> Result { let Config { program_id, operator, @@ -130,7 +126,7 @@ impl TransactionBuilder { Ok(this) } - pub async fn reload_config(&self) -> anyhow::Result<()> { + pub async fn reload_config(&self) -> Result<()> { let config = self.neon_api.get_config().await?; let treasury_pool_count: u32 = config @@ -172,12 +168,13 @@ impl TransactionBuilder { Ok(()) } - pub async fn recover(&mut self, init_holders: bool) -> anyhow::Result> { + pub async fn recover(&mut self, init_holders: bool) -> Result> { let mut output = Vec::new(); let holders = self.holder_mgr.recover().await; for holder in holders { let stage = match holder.state { + RecoverableHolderState::Recreate => TxStage::RecreateHolder { info: holder.info }, RecoverableHolderState::Pending(tx) => { let Some(chain_id) = get_chain_id(&tx) else { tracing::warn!( @@ -207,11 +204,15 @@ impl TransactionBuilder { .collect(); TxStage::recovered_holder(tx_hash, chain_id, holder.info, iter_info, accounts) } - RecoverableHolderState::State { chain_id: None, .. } => { + RecoverableHolderState::State { + chain_id: None, + tx_hash, + .. + } => { tracing::warn!( operator = %self.operator.pubkey(), pubkey = %holder.info.pubkey(), - tx_hash = %holder.state.tx_hash(), + %tx_hash, "cannot determine chain id for recovered holder" ); // TODO: Cancel @@ -264,7 +265,7 @@ impl TransactionBuilder { Pubkey::find_program_address(seeds, &self.program_id).0 } - pub fn treasury_pool(&self, hash: &B256) -> anyhow::Result<(u32, Pubkey)> { + pub fn treasury_pool(&self, hash: &B256) -> Result<(u32, Pubkey)> { let base_idx = u32::from_le_bytes(*hash.0.first_chunk().expect("B256 is longer than 4")); let evm_config = self.evm_config.load(); let treasury_pool_idx = base_idx % evm_config.treasury_pool_count; @@ -310,7 +311,7 @@ impl TransactionBuilder { /// ## Transaction flow. impl TransactionBuilder { - pub async fn start_execution(&self, tx: ExecuteRequest) -> anyhow::Result { + pub async fn start_execution(&self, tx: ExecuteRequest) -> Result { let chain_id = get_chain_id(&tx); let fits_in_solana_tx = Self::from_data_tx_len(tx.length(), 0) < PACKET_DATA_SIZE; @@ -328,15 +329,24 @@ impl TransactionBuilder { } } - pub async fn next_step( - &self, - tx: OngoingTransaction, - ) -> anyhow::Result> { + pub async fn next_step(&self, tx: OngoingTransaction) -> Result> { self.next_step_inner(tx.disassemble()).await } - async fn next_step_inner(&self, stage: TxStage) -> anyhow::Result> { + async fn next_step_inner(&self, stage: TxStage) -> Result> { match stage { + TxStage::RecreateHolder { info } => { + let ixs = [self.holder_mgr.delete_holder(&info)]; + Ok(Some( + TxStage::DeleteHolder { info }.ongoing(&ixs, &self.operator.pubkey()), + )) + } + TxStage::DeleteHolder { info } => { + let ixs = self.holder_mgr.create_holder(&info).await?; + Ok(Some( + TxStage::CreateHolder { info }.ongoing(&ixs, &self.operator.pubkey()), + )) + } TxStage::HolderFill { info, tx: envelope } if info.is_empty() => { self.execute_from_holder(info, envelope).await.map(Some) } @@ -375,6 +385,12 @@ impl TransactionBuilder { .await } TxStage::Final { .. } | TxStage::Cancel { .. } => Ok(None), + TxStage::CreateHolder { info } => { + // drop here explicitly for readability purposes; + // holder was recreated, we can now drop the info and return it back to the queue + drop(info); + Ok(None) + } } } diff --git a/executor/src/transactions/holder.rs b/executor/src/transactions/holder.rs index 81d0114..5223cab 100644 --- a/executor/src/transactions/holder.rs +++ b/executor/src/transactions/holder.rs @@ -124,6 +124,7 @@ impl HolderState { #[derive(Debug)] pub enum RecoverableHolderState { + Recreate, Pending(TxEnvelope), State { tx_hash: B256, @@ -132,19 +133,11 @@ pub enum RecoverableHolderState { }, } -impl RecoverableHolderState { - pub fn tx_hash(&self) -> &B256 { - match self { - Self::Pending(tx) => tx.tx_hash(), - Self::State { tx_hash, .. } => tx_hash, - } - } -} - #[derive(Debug)] struct RecoveredHolder { meta: HolderMeta, state: HolderState, + recreate: bool, } #[derive(Debug)] @@ -204,6 +197,12 @@ impl HolderManager { info!(%self.operator, ?recovered_holder, "discovered holder"); let info = self.attach_info(recovered_holder.meta, None); match recovered_holder.state.recoverable() { + None if recovered_holder.recreate => { + output.push(HolderToFinalize { + info, + state: RecoverableHolderState::Recreate, + }); + } None => drop(info), Some(state) => output.push(HolderToFinalize { info, state }), } @@ -259,6 +258,8 @@ impl HolderManager { let meta = HolderMeta { idx, pubkey }; let account_info = (&pubkey, &mut account).into_account_info(); + let holder_size = account_info.data.borrow().len(); + let recreate = holder_size != HOLDER_SIZE; let state = match tag(&self.program_id, &account_info).context("invalid holder account")? { account::TAG_STATE_FINALIZED | legacy::TAG_STATE_FINALIZED_DEPRECATED => { HolderState::Finalized @@ -296,9 +297,13 @@ impl HolderManager { accounts, } } - n => anyhow::bail!("invalid holder tag: {n}"), + n => bail!("invalid holder tag: {n}"), }; - Ok(Some(RecoveredHolder { meta, state })) + Ok(Some(RecoveredHolder { + meta, + state, + recreate, + })) } pub async fn acquire_holder(&self, tx: Option<&TxEnvelope>) -> anyhow::Result { @@ -350,14 +355,6 @@ impl HolderManager { )) } - pub async fn recreate_holder(&self, holder: &HolderInfo) -> anyhow::Result<[Instruction; 3]> { - // TODO: should we check if holder is in holder or finalized state? - let delete_ix = self.delete_holder(&holder.meta); - let create_ixs = self.create_holder(holder).await?; - let [create_ix0, create_ix1] = create_ixs; - Ok([delete_ix, create_ix0, create_ix1]) - } - fn holder_key(&self, idx: u8) -> Pubkey { let seed = holder_seed(idx); Pubkey::create_with_seed(&self.operator, &seed, &self.program_id) @@ -389,7 +386,7 @@ impl HolderManager { } } - async fn create_holder(&self, holder: &HolderInfo) -> anyhow::Result<[Instruction; 2]> { + pub async fn create_holder(&self, holder: &HolderInfo) -> anyhow::Result<[Instruction; 2]> { let seed = holder_seed(holder.meta.idx); let sp_ix = system_instruction::create_account_with_seed( &self.operator, @@ -427,10 +424,10 @@ impl HolderManager { Ok([sp_ix, neon_ix]) } - fn delete_holder(&self, meta: &HolderMeta) -> Instruction { + pub fn delete_holder(&self, holder: &HolderInfo) -> Instruction { let data = vec![tag::HOLDER_DELETE; 1]; let accounts = vec![ - AccountMeta::new(meta.pubkey, false), + AccountMeta::new(holder.meta.pubkey, false), AccountMeta::new_readonly(self.operator, true), ]; diff --git a/executor/src/transactions/ongoing.rs b/executor/src/transactions/ongoing.rs index 3ea4750..406f351 100644 --- a/executor/src/transactions/ongoing.rs +++ b/executor/src/transactions/ongoing.rs @@ -74,6 +74,15 @@ pub(super) enum TxStage { tx_hash: B256, _holder: HolderInfo, }, + RecreateHolder { + info: HolderInfo, + }, + CreateHolder { + info: HolderInfo, + }, + DeleteHolder { + info: HolderInfo, + }, } impl TxStage { @@ -290,7 +299,10 @@ impl OngoingTransaction { } => Some(&envelope.tx), TxStage::Final { tx_data: None, .. } | TxStage::RecoveredHolder { .. } - | TxStage::Cancel { .. } => None, + | TxStage::Cancel { .. } + | TxStage::RecreateHolder { .. } + | TxStage::CreateHolder { .. } + | TxStage::DeleteHolder { .. } => None, } } @@ -316,7 +328,10 @@ impl OngoingTransaction { TxStage::RecoveredHolder { tx_hash, .. } | TxStage::Cancel { tx_hash, .. } => { Some(tx_hash) } - TxStage::Final { tx_data: None, .. } => None, + TxStage::Final { tx_data: None, .. } + | TxStage::RecreateHolder { .. } + | TxStage::DeleteHolder { .. } + | TxStage::CreateHolder { .. } => None, } } @@ -332,7 +347,10 @@ impl OngoingTransaction { TxStage::Final { tx_data: None, .. } | TxStage::Cancel { .. } | TxStage::RecoveredHolder { .. } - | TxStage::HolderFill { .. } => false, + | TxStage::HolderFill { .. } + | TxStage::RecreateHolder { .. } + | TxStage::DeleteHolder { .. } + | TxStage::CreateHolder { .. } => false, } } @@ -368,7 +386,11 @@ impl OngoingTransaction { } | TxStage::HolderFill { ref tx, .. } => get_chain_id(&tx.tx), TxStage::RecoveredHolder { chain_id, .. } => Some(chain_id), - TxStage::Final { tx_data: None, .. } | TxStage::Cancel { .. } => None, + TxStage::Final { tx_data: None, .. } + | TxStage::Cancel { .. } + | TxStage::RecreateHolder { .. } + | TxStage::DeleteHolder { .. } + | TxStage::CreateHolder { .. } => None, } } diff --git a/executor/src/transactions/preflight_error.rs b/executor/src/transactions/preflight_error.rs index ae221b1..9ce4cdf 100644 --- a/executor/src/transactions/preflight_error.rs +++ b/executor/src/transactions/preflight_error.rs @@ -205,6 +205,9 @@ impl TransactionBuilder { | stage @ TxStage::Final { tx_data: None, .. } // Not an ETH tx | stage @ TxStage::Final { tx_data: Some(_), holder: Some(_) } // Has both holder and ALT | stage @ TxStage::RecoveredHolder { .. } + | stage @ TxStage::RecreateHolder { .. } + | stage @ TxStage::CreateHolder { .. } + | stage @ TxStage::DeleteHolder { .. } | stage @ TxStage::Cancel { .. } => bail!("cannot shorten tx size: {stage:?}"), } } @@ -339,7 +342,10 @@ impl TransactionBuilder { | stage @ TxStage::AltFill { .. } | stage @ TxStage::Final { .. } | stage @ TxStage::RecoveredHolder { .. } - | stage @ TxStage::Cancel { .. } => { + | stage @ TxStage::Cancel { .. } + | stage @ TxStage::RecreateHolder { .. } + | stage @ TxStage::CreateHolder { .. } + | stage @ TxStage::DeleteHolder { .. } => { bail!("{stage:?} cannot fallback to iterative") } }