From c056b751fefe1ad274c615e79a57d8b55dbf2f29 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 20 Oct 2023 16:12:26 -0400 Subject: [PATCH] Remove Fee from the Network API The only benefit to having it would be the ability to cache it across prepare_send, which can be done internally to the Network. --- processor/src/multisigs/mod.rs | 19 +++---------- processor/src/networks/bitcoin.rs | 46 ++++++++++++++----------------- processor/src/networks/mod.rs | 19 ++----------- processor/src/networks/monero.rs | 30 ++++++++------------ processor/src/tests/addresses.rs | 1 - processor/src/tests/signer.rs | 2 -- processor/src/tests/wallet.rs | 3 +- 7 files changed, 39 insertions(+), 81 deletions(-) diff --git a/processor/src/multisigs/mod.rs b/processor/src/multisigs/mod.rs index e511a8d8a..204684f24 100644 --- a/processor/src/multisigs/mod.rs +++ b/processor/src/multisigs/mod.rs @@ -36,9 +36,7 @@ use scheduler::Scheduler; use crate::{ Get, Db, Payment, Plan, - networks::{ - OutputType, Output, Transaction, SignableTransaction, Block, PreparedSend, Network, get_block, - }, + networks::{OutputType, Output, Transaction, SignableTransaction, Block, PreparedSend, Network}, }; // InInstructionWithBalance from an external output @@ -80,20 +78,14 @@ enum RotationStep { ClosingExisting, } -async fn get_fee_rate(network: &N, block_number: usize) -> N::Fee { - // TODO2: Use an fee representative of several blocks - get_block(network, block_number).await.median_fee() -} - async fn prepare_send( network: &N, block_number: usize, - fee_rate: N::Fee, plan: Plan, operating_costs: u64, ) -> PreparedSend { loop { - match network.prepare_send(block_number, plan.clone(), fee_rate, operating_costs).await { + match network.prepare_send(block_number, plan.clone(), operating_costs).await { Ok(prepared) => { return prepared; } @@ -157,15 +149,13 @@ impl MultisigManager { { let block_number = block_number.try_into().unwrap(); - let fee_rate = get_fee_rate(network, block_number).await; - let id = plan.id(); info!("reloading plan {}: {:?}", hex::encode(id), plan); let key_bytes = plan.key.to_bytes(); let Some((tx, eventuality)) = - prepare_send(network, block_number, fee_rate, plan.clone(), operating_costs).await.tx + prepare_send(network, block_number, plan.clone(), operating_costs).await.tx else { panic!("previously created transaction is no longer being created") }; @@ -675,7 +665,6 @@ impl MultisigManager { let res = { let mut res = Vec::with_capacity(plans.len()); - let fee_rate = get_fee_rate(network, block_number).await; for plan in plans { let id = plan.id(); @@ -699,7 +688,7 @@ impl MultisigManager { assert_eq!(plan.inputs.len(), 1); } let PreparedSend { tx, post_fee_branches, operating_costs } = - prepare_send(network, block_number, fee_rate, plan, running_operating_costs).await; + prepare_send(network, block_number, plan, running_operating_costs).await; // 'Drop' running_operating_costs to ensure only operating_costs is used from here on out #[allow(unused, clippy::let_unit_value)] let running_operating_costs: () = (); diff --git a/processor/src/networks/bitcoin.rs b/processor/src/networks/bitcoin.rs index 7ca5fcc24..2d6bc8acf 100644 --- a/processor/src/networks/bitcoin.rs +++ b/processor/src/networks/bitcoin.rs @@ -249,11 +249,6 @@ impl BlockTrait for Block { fn time(&self) -> u64 { self.header.time.into() } - - fn median_fee(&self) -> Fee { - // TODO - Fee(20) - } } const KEY_DST: &[u8] = b"Serai Bitcoin Output Offset"; @@ -322,12 +317,21 @@ impl Bitcoin { async fn make_signable_transaction( &self, + block_number: usize, inputs: &[Output], payments: &[Payment], change: &Option
, - fee: Fee, calculating_fee: bool, - ) -> Option { + ) -> Result, NetworkError> { + // TODO2: Use an fee representative of several blocks, cached inside Self + let block_for_fee = self.get_block(block_number).await?; + let median_fee = || { + // TODO + let _ = block_for_fee; + Fee(20) + }; + let fee = median_fee(); + let payments = payments .iter() .map(|payment| { @@ -348,22 +352,20 @@ impl Bitcoin { None, fee.0, ) { - Ok(signable) => Some(signable), + Ok(signable) => Ok(Some(signable)), Err(TransactionError::NoInputs) => { panic!("trying to create a bitcoin transaction without inputs") } // No outputs left and the change isn't worth enough - Err(TransactionError::NoOutputs) => None, + Err(TransactionError::NoOutputs) => Ok(None), // amortize_fee removes payments which fall below the dust threshold Err(TransactionError::DustPayment) => panic!("dust payment despite removing dust"), Err(TransactionError::TooMuchData) => panic!("too much data despite not specifying data"), Err(TransactionError::TooLowFee) => { panic!("created a transaction whose fee is below the minimum") } - Err(TransactionError::NotEnoughFunds) => { - // Mot even enough funds to pay the fee - None - } + // Mot even enough funds to pay the fee + Err(TransactionError::NotEnoughFunds) => Ok(None), Err(TransactionError::TooLargeTransaction) => { panic!("created a too large transaction despite limiting inputs/outputs") } @@ -375,7 +377,6 @@ impl Bitcoin { impl Network for Bitcoin { type Curve = Secp256k1; - type Fee = Fee; type Transaction = Transaction; type Block = Block; @@ -555,31 +556,29 @@ impl Network for Bitcoin { async fn needed_fee( &self, - _: usize, + block_number: usize, _: &[u8; 32], inputs: &[Output], payments: &[Payment], change: &Option
, - fee_rate: Fee, ) -> Result, NetworkError> { Ok( self - .make_signable_transaction(inputs, payments, change, fee_rate, true) - .await + .make_signable_transaction(block_number, inputs, payments, change, true) + .await? .map(|signable| signable.needed_fee()), ) } async fn signable_transaction( &self, - _: usize, + block_number: usize, plan_id: &[u8; 32], inputs: &[Output], payments: &[Payment], change: &Option
, - fee_rate: Fee, ) -> Result, NetworkError> { - Ok(self.make_signable_transaction(inputs, payments, change, fee_rate, false).await.map( + Ok(self.make_signable_transaction(block_number, inputs, payments, change, false).await?.map( |signable| { let mut transcript = RecommendedTranscript::new(b"Serai Processor Bitcoin Transaction Transcript"); @@ -635,11 +634,6 @@ impl Network for Bitcoin { self.rpc.get_block_number(id).await.unwrap() } - #[cfg(test)] - async fn get_fee(&self) -> Fee { - Fee(1) - } - #[cfg(test)] async fn mine_block(&self) { self diff --git a/processor/src/networks/mod.rs b/processor/src/networks/mod.rs index 3cc9c4aae..a350a015e 100644 --- a/processor/src/networks/mod.rs +++ b/processor/src/networks/mod.rs @@ -189,7 +189,6 @@ pub trait Block: Send + Sync + Sized + Clone + Debug { fn parent(&self) -> Self::Id; // The monotonic network time at this block. fn time(&self) -> u64; - fn median_fee(&self) -> N::Fee; } // The post-fee value of an expected branch. @@ -225,10 +224,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { /// The elliptic curve used for this network. type Curve: Curve; - /// The type representing the fee for this network. - // This should likely be a u64, wrapped in a type which implements appropriate fee logic. - type Fee: Send + Copy; - /// The type representing the transaction for this network. type Transaction: Transaction; /// The type representing the block for this network. @@ -322,7 +317,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { inputs: &[Self::Output], payments: &[Payment], change: &Option, - fee_rate: Self::Fee, ) -> Result, NetworkError>; /// Create a SignableTransaction for the given Plan. @@ -338,7 +332,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { inputs: &[Self::Output], payments: &[Payment], change: &Option, - fee_rate: Self::Fee, ) -> Result, NetworkError>; /// Prepare a SignableTransaction for a transaction. @@ -346,7 +339,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { &self, block_number: usize, plan: Plan, - fee_rate: Self::Fee, operating_costs: u64, ) -> Result, NetworkError> { // Sanity check this has at least one output planned @@ -357,8 +349,7 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { let theoretical_change_amount = inputs.iter().map(|input| input.amount()).sum::() - payments.iter().map(|payment| payment.amount).sum::(); - let Some(tx_fee) = - self.needed_fee(block_number, &plan_id, &inputs, &payments, &change, fee_rate).await? + let Some(tx_fee) = self.needed_fee(block_number, &plan_id, &inputs, &payments, &change).await? else { // This Plan is not fulfillable // TODO: Have Plan explicitly distinguish payments and branches in two separate Vecs? @@ -462,9 +453,8 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { ) })(); - let Some(tx) = self - .signable_transaction(block_number, &plan_id, &inputs, &payments, &change, fee_rate) - .await? + let Some(tx) = + self.signable_transaction(block_number, &plan_id, &inputs, &payments, &change).await? else { panic!( "{}. {}: {}, {}: {:?}, {}: {:?}, {}: {:?}, {}: {}", @@ -525,9 +515,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { #[cfg(test)] async fn get_block_number(&self, id: &>::Id) -> usize; - #[cfg(test)] - async fn get_fee(&self) -> Self::Fee; - #[cfg(test)] async fn mine_block(&self); diff --git a/processor/src/networks/monero.rs b/processor/src/networks/monero.rs index 06b8a7400..f9cae96d8 100644 --- a/processor/src/networks/monero.rs +++ b/processor/src/networks/monero.rs @@ -161,11 +161,6 @@ impl BlockTrait for Block { fn time(&self) -> u64 { self.header.timestamp } - - fn median_fee(&self) -> Fee { - // TODO - Fee { per_weight: 10000000, mask: 10000 } - } } #[derive(Clone, Debug)] @@ -206,7 +201,6 @@ impl Monero { scanner } - #[allow(clippy::too_many_arguments)] async fn make_signable_transaction( &self, block_number: usize, @@ -214,9 +208,17 @@ impl Monero { inputs: &[Output], payments: &[Payment], change: &Option
, - fee_rate: Fee, calculating_fee: bool, ) -> Result, NetworkError> { + // TODO2: Use an fee representative of several blocks, cached inside Self + let block_for_fee = self.get_block(block_number).await?; + let median_fee = || { + // TODO + let _ = block_for_fee; + Fee { per_weight: 10000000, mask: 10000 } + }; + let fee_rate = median_fee(); + // Get the protocol for the specified block number // For now, this should just be v16, the latest deployed protocol, since there's no upcoming // hard fork to be mindful of @@ -352,7 +354,6 @@ impl Monero { impl Network for Monero { type Curve = Ed25519; - type Fee = Fee; type Transaction = Transaction; type Block = Block; @@ -525,11 +526,10 @@ impl Network for Monero { inputs: &[Output], payments: &[Payment], change: &Option
, - fee_rate: Fee, ) -> Result, NetworkError> { Ok( self - .make_signable_transaction(block_number, plan_id, inputs, payments, change, fee_rate, true) + .make_signable_transaction(block_number, plan_id, inputs, payments, change, true) .await? .map(|(_, signable)| signable.fee()), ) @@ -542,11 +542,10 @@ impl Network for Monero { inputs: &[Output], payments: &[Payment], change: &Option
, - fee_rate: Fee, ) -> Result, NetworkError> { Ok( self - .make_signable_transaction(block_number, plan_id, inputs, payments, change, fee_rate, false) + .make_signable_transaction(block_number, plan_id, inputs, payments, change, false) .await? .map(|(transcript, signable)| { let signable = SignableTransaction { transcript, actual: signable }; @@ -590,13 +589,6 @@ impl Network for Monero { self.rpc.get_block(*id).await.unwrap().number() } - #[cfg(test)] - async fn get_fee(&self) -> Fee { - use monero_serai::wallet::FeePriority; - - self.rpc.get_fee(self.rpc.get_protocol().await.unwrap(), FeePriority::Low).await.unwrap() - } - #[cfg(test)] async fn mine_block(&self) { // https://github.com/serai-dex/serai/issues/198 diff --git a/processor/src/tests/addresses.rs b/processor/src/tests/addresses.rs index 44b470a6d..f9c621ee6 100644 --- a/processor/src/tests/addresses.rs +++ b/processor/src/tests/addresses.rs @@ -41,7 +41,6 @@ async fn spend( payments: vec![], change: Some(N::change_address(key)), }, - network.get_fee().await, 0, ) .await diff --git a/processor/src/tests/signer.rs b/processor/src/tests/signer.rs index 88c4b5e3d..b8f197766 100644 --- a/processor/src/tests/signer.rs +++ b/processor/src/tests/signer.rs @@ -155,7 +155,6 @@ pub async fn test_signer(network: N) { let outputs = network.get_outputs(&network.test_send(N::address(key)).await, key).await; let sync_block = network.get_latest_block_number().await.unwrap() - N::CONFIRMATIONS; - let fee = network.get_fee().await; let amount = 2 * N::DUST; let mut keys_txs = HashMap::new(); @@ -170,7 +169,6 @@ pub async fn test_signer(network: N) { payments: vec![Payment { address: N::address(key), data: None, amount }], change: Some(N::change_address(key)), }, - fee, 0, ) .await diff --git a/processor/src/tests/wallet.rs b/processor/src/tests/wallet.rs index aaf777b12..fa88d784c 100644 --- a/processor/src/tests/wallet.rs +++ b/processor/src/tests/wallet.rs @@ -91,12 +91,11 @@ pub async fn test_wallet(network: N) { } // Execute the plan - let fee = network.get_fee().await; let mut keys_txs = HashMap::new(); let mut eventualities = vec![]; for (i, keys) in keys.drain() { let (signable, eventuality) = network - .prepare_send(network.get_block_number(&block_id).await, plans[0].clone(), fee, 0) + .prepare_send(network.get_block_number(&block_id).await, plans[0].clone(), 0) .await .unwrap() .tx