Skip to content

Commit

Permalink
Remove Fee from the Network API
Browse files Browse the repository at this point in the history
The only benefit to having it would be the ability to cache it across
prepare_send, which can be done internally to the Network.
  • Loading branch information
kayabaNerve committed Oct 20, 2023
1 parent 5977121 commit c056b75
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 81 deletions.
19 changes: 4 additions & 15 deletions processor/src/multisigs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -80,20 +78,14 @@ enum RotationStep {
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>(
network: &N,
block_number: usize,
fee_rate: N::Fee,
plan: Plan<N>,
operating_costs: u64,
) -> PreparedSend<N> {
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;
}
Expand Down Expand Up @@ -157,15 +149,13 @@ impl<D: Db, N: Network> MultisigManager<D, N> {
{
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")
};
Expand Down Expand Up @@ -675,7 +665,6 @@ impl<D: Db, N: Network> MultisigManager<D, N> {

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();
Expand All @@ -699,7 +688,7 @@ impl<D: Db, N: Network> MultisigManager<D, N> {
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: () = ();
Expand Down
46 changes: 20 additions & 26 deletions processor/src/networks/bitcoin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,6 @@ impl BlockTrait<Bitcoin> 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";
Expand Down Expand Up @@ -322,12 +317,21 @@ impl Bitcoin {

async fn make_signable_transaction(
&self,
block_number: usize,
inputs: &[Output],
payments: &[Payment<Self>],
change: &Option<Address>,
fee: Fee,
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
.iter()
.map(|payment| {
Expand All @@ -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")
}
Expand All @@ -375,7 +377,6 @@ impl Bitcoin {
impl Network for Bitcoin {
type Curve = Secp256k1;

type Fee = Fee;
type Transaction = Transaction;
type Block = Block;

Expand Down Expand Up @@ -555,31 +556,29 @@ impl Network for Bitcoin {

async fn needed_fee(
&self,
_: usize,
block_number: usize,
_: &[u8; 32],
inputs: &[Output],
payments: &[Payment<Self>],
change: &Option<Address>,
fee_rate: Fee,
) -> Result<Option<u64>, 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<Self>],
change: &Option<Address>,
fee_rate: Fee,
) -> 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| {
let mut transcript =
RecommendedTranscript::new(b"Serai Processor Bitcoin Transaction Transcript");
Expand Down Expand Up @@ -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
Expand Down
19 changes: 3 additions & 16 deletions processor/src/networks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ pub trait Block<N: Network>: 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.
Expand Down Expand Up @@ -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<Self>;
/// The type representing the block for this network.
Expand Down Expand Up @@ -322,7 +317,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
inputs: &[Self::Output],
payments: &[Payment<Self>],
change: &Option<Self::Address>,
fee_rate: Self::Fee,
) -> Result<Option<u64>, NetworkError>;

/// Create a SignableTransaction for the given Plan.
Expand All @@ -338,15 +332,13 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
inputs: &[Self::Output],
payments: &[Payment<Self>],
change: &Option<Self::Address>,
fee_rate: Self::Fee,
) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, NetworkError>;

/// Prepare a SignableTransaction for a transaction.
async fn prepare_send(
&self,
block_number: usize,
plan: Plan<Self>,
fee_rate: Self::Fee,
operating_costs: u64,
) -> Result<PreparedSend<Self>, NetworkError> {
// Sanity check this has at least one output planned
Expand All @@ -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>() -
payments.iter().map(|payment| payment.amount).sum::<u64>();

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?
Expand Down Expand Up @@ -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!(
"{}. {}: {}, {}: {:?}, {}: {:?}, {}: {:?}, {}: {}",
Expand Down Expand Up @@ -525,9 +515,6 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
#[cfg(test)]
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)]
async fn mine_block(&self);

Expand Down
30 changes: 11 additions & 19 deletions processor/src/networks/monero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,6 @@ impl BlockTrait<Monero> for Block {
fn time(&self) -> u64 {
self.header.timestamp
}

fn median_fee(&self) -> Fee {
// TODO
Fee { per_weight: 10000000, mask: 10000 }
}
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -206,17 +201,24 @@ impl Monero {
scanner
}

#[allow(clippy::too_many_arguments)]
async fn make_signable_transaction(
&self,
block_number: usize,
plan_id: &[u8; 32],
inputs: &[Output],
payments: &[Payment<Self>],
change: &Option<Address>,
fee_rate: Fee,
calculating_fee: bool,
) -> 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
// For now, this should just be v16, the latest deployed protocol, since there's no upcoming
// hard fork to be mindful of
Expand Down Expand Up @@ -352,7 +354,6 @@ impl Monero {
impl Network for Monero {
type Curve = Ed25519;

type Fee = Fee;
type Transaction = Transaction;
type Block = Block;

Expand Down Expand Up @@ -525,11 +526,10 @@ impl Network for Monero {
inputs: &[Output],
payments: &[Payment<Self>],
change: &Option<Address>,
fee_rate: Fee,
) -> Result<Option<u64>, 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()),
)
Expand All @@ -542,11 +542,10 @@ impl Network for Monero {
inputs: &[Output],
payments: &[Payment<Self>],
change: &Option<Address>,
fee_rate: Fee,
) -> Result<Option<(Self::SignableTransaction, Self::Eventuality)>, 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 };
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion processor/src/tests/addresses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ async fn spend<N: Network, D: Db>(
payments: vec![],
change: Some(N::change_address(key)),
},
network.get_fee().await,
0,
)
.await
Expand Down
2 changes: 0 additions & 2 deletions processor/src/tests/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 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();
Expand All @@ -170,7 +169,6 @@ pub async fn test_signer<N: Network>(network: N) {
payments: vec![Payment { address: N::address(key), data: None, amount }],
change: Some(N::change_address(key)),
},
fee,
0,
)
.await
Expand Down
3 changes: 1 addition & 2 deletions processor/src/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,11 @@ pub async fn test_wallet<N: Network>(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
Expand Down

0 comments on commit c056b75

Please sign in to comment.