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.
This commit is contained in:
Luke Parker
2023-10-20 16:12:26 -04:00
parent 5977121c48
commit c056b751fe
7 changed files with 39 additions and 81 deletions

View File

@@ -36,9 +36,7 @@ use scheduler::Scheduler;
use crate::{ use crate::{
Get, Db, Payment, Plan, Get, Db, Payment, Plan,
networks::{ networks::{OutputType, Output, Transaction, SignableTransaction, Block, PreparedSend, Network},
OutputType, Output, Transaction, SignableTransaction, Block, PreparedSend, Network, get_block,
},
}; };
// InInstructionWithBalance from an external output // InInstructionWithBalance from an external output
@@ -80,20 +78,14 @@ enum RotationStep {
ClosingExisting, ClosingExisting,
} }
async fn get_fee_rate<N: Network>(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<N: Network>( async fn prepare_send<N: Network>(
network: &N, network: &N,
block_number: usize, block_number: usize,
fee_rate: N::Fee,
plan: Plan<N>, plan: Plan<N>,
operating_costs: u64, operating_costs: u64,
) -> PreparedSend<N> { ) -> PreparedSend<N> {
loop { 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) => { Ok(prepared) => {
return prepared; return prepared;
} }
@@ -157,15 +149,13 @@ impl<D: Db, N: Network> MultisigManager<D, N> {
{ {
let block_number = block_number.try_into().unwrap(); let block_number = block_number.try_into().unwrap();
let fee_rate = get_fee_rate(network, block_number).await;
let id = plan.id(); let id = plan.id();
info!("reloading plan {}: {:?}", hex::encode(id), plan); info!("reloading plan {}: {:?}", hex::encode(id), plan);
let key_bytes = plan.key.to_bytes(); let key_bytes = plan.key.to_bytes();
let Some((tx, eventuality)) = 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 { else {
panic!("previously created transaction is no longer being created") panic!("previously created transaction is no longer being created")
}; };
@@ -675,7 +665,6 @@ impl<D: Db, N: Network> MultisigManager<D, N> {
let res = { let res = {
let mut res = Vec::with_capacity(plans.len()); let mut res = Vec::with_capacity(plans.len());
let fee_rate = get_fee_rate(network, block_number).await;
for plan in plans { for plan in plans {
let id = plan.id(); let id = plan.id();
@@ -699,7 +688,7 @@ impl<D: Db, N: Network> MultisigManager<D, N> {
assert_eq!(plan.inputs.len(), 1); assert_eq!(plan.inputs.len(), 1);
} }
let PreparedSend { tx, post_fee_branches, operating_costs } = 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 // 'Drop' running_operating_costs to ensure only operating_costs is used from here on out
#[allow(unused, clippy::let_unit_value)] #[allow(unused, clippy::let_unit_value)]
let running_operating_costs: () = (); let running_operating_costs: () = ();

View File

@@ -249,11 +249,6 @@ impl BlockTrait<Bitcoin> for Block {
fn time(&self) -> u64 { fn time(&self) -> u64 {
self.header.time.into() self.header.time.into()
} }
fn median_fee(&self) -> Fee {
// TODO
Fee(20)
}
} }
const KEY_DST: &[u8] = b"Serai Bitcoin Output Offset"; const KEY_DST: &[u8] = b"Serai Bitcoin Output Offset";
@@ -322,12 +317,21 @@ impl Bitcoin {
async fn make_signable_transaction( async fn make_signable_transaction(
&self, &self,
block_number: usize,
inputs: &[Output], inputs: &[Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Address>, change: &Option<Address>,
fee: Fee,
calculating_fee: bool, calculating_fee: bool,
) -> Option<BSignableTransaction> { ) -> Result<Option<BSignableTransaction>, 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 let payments = payments
.iter() .iter()
.map(|payment| { .map(|payment| {
@@ -348,22 +352,20 @@ impl Bitcoin {
None, None,
fee.0, fee.0,
) { ) {
Ok(signable) => Some(signable), Ok(signable) => Ok(Some(signable)),
Err(TransactionError::NoInputs) => { Err(TransactionError::NoInputs) => {
panic!("trying to create a bitcoin transaction without inputs") panic!("trying to create a bitcoin transaction without inputs")
} }
// No outputs left and the change isn't worth enough // 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 // amortize_fee removes payments which fall below the dust threshold
Err(TransactionError::DustPayment) => panic!("dust payment despite removing dust"), Err(TransactionError::DustPayment) => panic!("dust payment despite removing dust"),
Err(TransactionError::TooMuchData) => panic!("too much data despite not specifying data"), Err(TransactionError::TooMuchData) => panic!("too much data despite not specifying data"),
Err(TransactionError::TooLowFee) => { Err(TransactionError::TooLowFee) => {
panic!("created a transaction whose fee is below the minimum") panic!("created a transaction whose fee is below the minimum")
} }
Err(TransactionError::NotEnoughFunds) => { // Mot even enough funds to pay the fee
// Mot even enough funds to pay the fee Err(TransactionError::NotEnoughFunds) => Ok(None),
None
}
Err(TransactionError::TooLargeTransaction) => { Err(TransactionError::TooLargeTransaction) => {
panic!("created a too large transaction despite limiting inputs/outputs") panic!("created a too large transaction despite limiting inputs/outputs")
} }
@@ -375,7 +377,6 @@ impl Bitcoin {
impl Network for Bitcoin { impl Network for Bitcoin {
type Curve = Secp256k1; type Curve = Secp256k1;
type Fee = Fee;
type Transaction = Transaction; type Transaction = Transaction;
type Block = Block; type Block = Block;
@@ -555,31 +556,29 @@ impl Network for Bitcoin {
async fn needed_fee( async fn needed_fee(
&self, &self,
_: usize, block_number: usize,
_: &[u8; 32], _: &[u8; 32],
inputs: &[Output], inputs: &[Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Address>, change: &Option<Address>,
fee_rate: Fee,
) -> Result<Option<u64>, NetworkError> { ) -> Result<Option<u64>, NetworkError> {
Ok( Ok(
self self
.make_signable_transaction(inputs, payments, change, fee_rate, true) .make_signable_transaction(block_number, inputs, payments, change, true)
.await .await?
.map(|signable| signable.needed_fee()), .map(|signable| signable.needed_fee()),
) )
} }
async fn signable_transaction( async fn signable_transaction(
&self, &self,
_: usize, block_number: usize,
plan_id: &[u8; 32], plan_id: &[u8; 32],
inputs: &[Output], inputs: &[Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Address>, change: &Option<Address>,
fee_rate: Fee,
) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, NetworkError> { ) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, 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| { |signable| {
let mut transcript = let mut transcript =
RecommendedTranscript::new(b"Serai Processor Bitcoin Transaction 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() self.rpc.get_block_number(id).await.unwrap()
} }
#[cfg(test)]
async fn get_fee(&self) -> Fee {
Fee(1)
}
#[cfg(test)] #[cfg(test)]
async fn mine_block(&self) { async fn mine_block(&self) {
self self

View File

@@ -189,7 +189,6 @@ pub trait Block<N: Network>: Send + Sync + Sized + Clone + Debug {
fn parent(&self) -> Self::Id; fn parent(&self) -> Self::Id;
// The monotonic network time at this block. // The monotonic network time at this block.
fn time(&self) -> u64; fn time(&self) -> u64;
fn median_fee(&self) -> N::Fee;
} }
// The post-fee value of an expected branch. // 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. /// The elliptic curve used for this network.
type Curve: Curve; 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. /// The type representing the transaction for this network.
type Transaction: Transaction<Self>; type Transaction: Transaction<Self>;
/// The type representing the block for this network. /// 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], inputs: &[Self::Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Self::Address>, change: &Option<Self::Address>,
fee_rate: Self::Fee,
) -> Result<Option<u64>, NetworkError>; ) -> Result<Option<u64>, NetworkError>;
/// Create a SignableTransaction for the given Plan. /// Create a SignableTransaction for the given Plan.
@@ -338,7 +332,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
inputs: &[Self::Output], inputs: &[Self::Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Self::Address>, change: &Option<Self::Address>,
fee_rate: Self::Fee,
) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, NetworkError>; ) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, NetworkError>;
/// Prepare a SignableTransaction for a transaction. /// Prepare a SignableTransaction for a transaction.
@@ -346,7 +339,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
&self, &self,
block_number: usize, block_number: usize,
plan: Plan<Self>, plan: Plan<Self>,
fee_rate: Self::Fee,
operating_costs: u64, operating_costs: u64,
) -> Result<PreparedSend<Self>, NetworkError> { ) -> Result<PreparedSend<Self>, NetworkError> {
// Sanity check this has at least one output planned // 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::<u64>() - let theoretical_change_amount = inputs.iter().map(|input| input.amount()).sum::<u64>() -
payments.iter().map(|payment| payment.amount).sum::<u64>(); payments.iter().map(|payment| payment.amount).sum::<u64>();
let Some(tx_fee) = let Some(tx_fee) = self.needed_fee(block_number, &plan_id, &inputs, &payments, &change).await?
self.needed_fee(block_number, &plan_id, &inputs, &payments, &change, fee_rate).await?
else { else {
// This Plan is not fulfillable // This Plan is not fulfillable
// TODO: Have Plan explicitly distinguish payments and branches in two separate Vecs? // 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 let Some(tx) =
.signable_transaction(block_number, &plan_id, &inputs, &payments, &change, fee_rate) self.signable_transaction(block_number, &plan_id, &inputs, &payments, &change).await?
.await?
else { else {
panic!( panic!(
"{}. {}: {}, {}: {:?}, {}: {:?}, {}: {:?}, {}: {}", "{}. {}: {}, {}: {:?}, {}: {:?}, {}: {:?}, {}: {}",
@@ -525,9 +515,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
#[cfg(test)] #[cfg(test)]
async fn get_block_number(&self, id: &<Self::Block as Block<Self>>::Id) -> usize; async fn get_block_number(&self, id: &<Self::Block as Block<Self>>::Id) -> usize;
#[cfg(test)]
async fn get_fee(&self) -> Self::Fee;
#[cfg(test)] #[cfg(test)]
async fn mine_block(&self); async fn mine_block(&self);

View File

@@ -161,11 +161,6 @@ impl BlockTrait<Monero> for Block {
fn time(&self) -> u64 { fn time(&self) -> u64 {
self.header.timestamp self.header.timestamp
} }
fn median_fee(&self) -> Fee {
// TODO
Fee { per_weight: 10000000, mask: 10000 }
}
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
@@ -206,7 +201,6 @@ impl Monero {
scanner scanner
} }
#[allow(clippy::too_many_arguments)]
async fn make_signable_transaction( async fn make_signable_transaction(
&self, &self,
block_number: usize, block_number: usize,
@@ -214,9 +208,17 @@ impl Monero {
inputs: &[Output], inputs: &[Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Address>, change: &Option<Address>,
fee_rate: Fee,
calculating_fee: bool, calculating_fee: bool,
) -> Result<Option<(RecommendedTranscript, MSignableTransaction)>, NetworkError> { ) -> Result<Option<(RecommendedTranscript, MSignableTransaction)>, 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 // Get the protocol for the specified block number
// For now, this should just be v16, the latest deployed protocol, since there's no upcoming // For now, this should just be v16, the latest deployed protocol, since there's no upcoming
// hard fork to be mindful of // hard fork to be mindful of
@@ -352,7 +354,6 @@ impl Monero {
impl Network for Monero { impl Network for Monero {
type Curve = Ed25519; type Curve = Ed25519;
type Fee = Fee;
type Transaction = Transaction; type Transaction = Transaction;
type Block = Block; type Block = Block;
@@ -525,11 +526,10 @@ impl Network for Monero {
inputs: &[Output], inputs: &[Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Address>, change: &Option<Address>,
fee_rate: Fee,
) -> Result<Option<u64>, NetworkError> { ) -> Result<Option<u64>, NetworkError> {
Ok( Ok(
self 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? .await?
.map(|(_, signable)| signable.fee()), .map(|(_, signable)| signable.fee()),
) )
@@ -542,11 +542,10 @@ impl Network for Monero {
inputs: &[Output], inputs: &[Output],
payments: &[Payment<Self>], payments: &[Payment<Self>],
change: &Option<Address>, change: &Option<Address>,
fee_rate: Fee,
) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, NetworkError> { ) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, NetworkError> {
Ok( Ok(
self 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? .await?
.map(|(transcript, signable)| { .map(|(transcript, signable)| {
let signable = SignableTransaction { transcript, actual: signable }; let signable = SignableTransaction { transcript, actual: signable };
@@ -590,13 +589,6 @@ impl Network for Monero {
self.rpc.get_block(*id).await.unwrap().number() 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)] #[cfg(test)]
async fn mine_block(&self) { async fn mine_block(&self) {
// https://github.com/serai-dex/serai/issues/198 // https://github.com/serai-dex/serai/issues/198

View File

@@ -41,7 +41,6 @@ async fn spend<N: Network, D: Db>(
payments: vec![], payments: vec![],
change: Some(N::change_address(key)), change: Some(N::change_address(key)),
}, },
network.get_fee().await,
0, 0,
) )
.await .await

View File

@@ -155,7 +155,6 @@ pub async fn test_signer<N: Network>(network: N) {
let outputs = network.get_outputs(&network.test_send(N::address(key)).await, key).await; 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 sync_block = network.get_latest_block_number().await.unwrap() - N::CONFIRMATIONS;
let fee = network.get_fee().await;
let amount = 2 * N::DUST; let amount = 2 * N::DUST;
let mut keys_txs = HashMap::new(); let mut keys_txs = HashMap::new();
@@ -170,7 +169,6 @@ pub async fn test_signer<N: Network>(network: N) {
payments: vec![Payment { address: N::address(key), data: None, amount }], payments: vec![Payment { address: N::address(key), data: None, amount }],
change: Some(N::change_address(key)), change: Some(N::change_address(key)),
}, },
fee,
0, 0,
) )
.await .await

View File

@@ -91,12 +91,11 @@ pub async fn test_wallet<N: Network>(network: N) {
} }
// Execute the plan // Execute the plan
let fee = network.get_fee().await;
let mut keys_txs = HashMap::new(); let mut keys_txs = HashMap::new();
let mut eventualities = vec![]; let mut eventualities = vec![];
for (i, keys) in keys.drain() { for (i, keys) in keys.drain() {
let (signable, eventuality) = network 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 .await
.unwrap() .unwrap()
.tx .tx