diff --git a/docs/processor/UTXO Management.md b/docs/processor/UTXO Management.md index 130f8ce3..8dd20450 100644 --- a/docs/processor/UTXO Management.md +++ b/docs/processor/UTXO Management.md @@ -130,7 +130,7 @@ Input accumulation refers to transactions which exist to merge inputs. Just as there is a `max_outputs_per_tx`, there is a `max_inputs_per_tx`. When the amount of inputs belonging to Serai exceeds `max_inputs_per_tx`, a TX merging them is created. This TX incurs fees yet has no outputs mapping to burns to amortize -them over, creating an insolvency. +them over, accumulating operating costs. Please note that this merging occurs in parallel to create a logarithmic execution, similar to how outputs are also processed in parallel. @@ -154,18 +154,16 @@ initially filled, yet requires: while still risking insolvency, if the actual fees keep increasing in a way preventing successful estimation. -The solution Serai implements is to accrue insolvency, tracking each output with -a virtual amount (the amount it represents on Serai) and the actual amount. When -the output, or a descendant of it, is used to handle burns, the discrepancy -between the virtual amount and the amount is amortized over outputs. This -restores solvency while solely charging the actual fees, making Serai a -generally insolvent, always eventually solvent system. +The solution Serai implements is to accrue operating costs, tracking with each +created transaction the running operating costs. When a created transaction has +payments out, all of the operating costs incurred so far, which have yet to be +amortized, are immediately and fully amortized. There is the concern that a significant amount of outputs could be created, -which when merged as inputs, create a significant amount of fees as an -insolvency. This would then be forced onto random users, while the party who -created the insolvency would then be able to burn their own `sriXYZ` without -the notable insolvency. +which when merged as inputs, create a significant amount of operating costs. +This would then be forced onto random users who burn `sriXYZ` soon after, while +the party who caused the operating costs would then be able to burn their own +`sriXYZ` without notable fees. To describe this attack in its optimal form, assume a sole malicious block producer for an external network where `max_inputs_per_tx` is 16. The malicious diff --git a/processor/src/main.rs b/processor/src/main.rs index 18a1428c..19b0ed06 100644 --- a/processor/src/main.rs +++ b/processor/src/main.rs @@ -23,7 +23,7 @@ mod plan; pub use plan::*; mod networks; -use networks::{PostFeeBranch, Block, Network, get_latest_block_number, get_block}; +use networks::{Block, Network, get_latest_block_number, get_block}; #[cfg(feature = "bitcoin")] use networks::Bitcoin; #[cfg(feature = "monero")] diff --git a/processor/src/multisigs/db.rs b/processor/src/multisigs/db.rs index 353bc4f8..4fde8dc3 100644 --- a/processor/src/multisigs/db.rs +++ b/processor/src/multisigs/db.rs @@ -45,6 +45,7 @@ impl MultisigsDb { key: &[u8], block_number: u64, plan: &Plan, + operating_costs_at_time: u64, ) { let id = plan.id(); @@ -66,11 +67,12 @@ impl MultisigsDb { { let mut buf = block_number.to_le_bytes().to_vec(); plan.write(&mut buf).unwrap(); + buf.extend(&operating_costs_at_time.to_le_bytes()); txn.put(Self::plan_key(&id), &buf); } } - pub fn active_plans(getter: &G, key: &[u8]) -> Vec<(u64, Plan)> { + pub fn active_plans(getter: &G, key: &[u8]) -> Vec<(u64, Plan, u64)> { let signing = getter.get(Self::signing_key(key)).unwrap_or(vec![]); let mut res = vec![]; @@ -82,12 +84,30 @@ impl MultisigsDb { let block_number = u64::from_le_bytes(buf[.. 8].try_into().unwrap()); let plan = Plan::::read::<&[u8]>(&mut &buf[8 ..]).unwrap(); assert_eq!(id, &plan.id()); - res.push((block_number, plan)); + let operating_costs = u64::from_le_bytes(buf[(buf.len() - 8) ..].try_into().unwrap()); + res.push((block_number, plan, operating_costs)); } res } + fn operating_costs_key() -> Vec { + Self::multisigs_key(b"operating_costs", []) + } + pub fn take_operating_costs(txn: &mut D::Transaction<'_>) -> u64 { + let existing = txn + .get(Self::operating_costs_key()) + .map(|bytes| u64::from_le_bytes(bytes.try_into().unwrap())) + .unwrap_or(0); + txn.del(Self::operating_costs_key()); + existing + } + pub fn set_operating_costs(txn: &mut D::Transaction<'_>, amount: u64) { + if amount != 0 { + txn.put(Self::operating_costs_key(), amount.to_le_bytes()); + } + } + pub fn resolved_plan( getter: &G, tx: >::Id, diff --git a/processor/src/multisigs/mod.rs b/processor/src/multisigs/mod.rs index 0071df6c..78bff506 100644 --- a/processor/src/multisigs/mod.rs +++ b/processor/src/multisigs/mod.rs @@ -35,8 +35,10 @@ pub mod scheduler; use scheduler::Scheduler; use crate::{ - Get, Db, Payment, PostFeeBranch, Plan, - networks::{OutputType, Output, Transaction, SignableTransaction, Block, Network, get_block}, + Get, Db, Payment, Plan, + networks::{ + OutputType, Output, Transaction, SignableTransaction, Block, PreparedSend, Network, get_block, + }, }; // InInstructionWithBalance from an external output @@ -57,8 +59,12 @@ fn instruction_from_output(output: &N::Output) -> Option(network: &N, block_number: usize) -> N::Fee { +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() } @@ -82,11 +88,12 @@ async fn get_fee(network: &N, block_number: usize) -> N::Fee { async fn prepare_send( network: &N, block_number: usize, - fee: N::Fee, + fee_rate: N::Fee, plan: Plan, -) -> (Option<(N::SignableTransaction, N::Eventuality)>, Vec) { + operating_costs: u64, +) -> PreparedSend { loop { - match network.prepare_send(block_number, plan.clone(), fee).await { + match network.prepare_send(block_number, plan.clone(), fee_rate, operating_costs).await { Ok(prepared) => { return prepared; } @@ -145,18 +152,20 @@ impl MultisigManager { // Load any TXs being actively signed let key = key.to_bytes(); - for (block_number, plan) in MultisigsDb::::active_plans(raw_db, key.as_ref()) { + for (block_number, plan, operating_costs) in + MultisigsDb::::active_plans(raw_db, key.as_ref()) + { let block_number = block_number.try_into().unwrap(); - let fee = get_fee(network, block_number).await; + 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, plan.clone()).await + let Some((tx, eventuality)) = + prepare_send(network, block_number, fee_rate, plan.clone(), operating_costs).await.tx else { panic!("previously created transaction is no longer being created") }; @@ -666,7 +675,7 @@ impl MultisigManager { let res = { let mut res = Vec::with_capacity(plans.len()); - let fee = get_fee(network, block_number).await; + let fee_rate = get_fee_rate(network, block_number).await; for plan in plans { let id = plan.id(); @@ -674,18 +683,27 @@ impl MultisigManager { let key = plan.key; let key_bytes = key.to_bytes(); + + let running_operating_costs = MultisigsDb::::take_operating_costs(txn); + MultisigsDb::::save_active_plan( txn, key_bytes.as_ref(), block_number.try_into().unwrap(), &plan, + running_operating_costs, ); let to_be_forwarded = forwarded_external_outputs.remove(plan.inputs[0].id().as_ref()); if to_be_forwarded.is_some() { assert_eq!(plan.inputs.len(), 1); } - let (tx, branches) = prepare_send(network, block_number, fee, plan).await; + let PreparedSend { tx, post_fee_branches, operating_costs } = + prepare_send(network, block_number, fee_rate, 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: () = (); + MultisigsDb::::set_operating_costs(txn, operating_costs); // If this is a Plan for an output we're forwarding, we need to save the InInstruction for // its output under the amount successfully forwarded @@ -697,7 +715,7 @@ impl MultisigManager { } } - for branch in branches { + for branch in post_fee_branches { let existing = self.existing.as_mut().unwrap(); let to_use = if key == existing.key { existing diff --git a/processor/src/multisigs/scheduler.rs b/processor/src/multisigs/scheduler.rs index 4d0e42ff..f2d41c09 100644 --- a/processor/src/multisigs/scheduler.rs +++ b/processor/src/multisigs/scheduler.rs @@ -322,8 +322,6 @@ impl Scheduler { } for chunk in utxo_chunks.drain(..) { - // TODO: While payments have their TXs' fees deducted from themselves, that doesn't hold here - // We need the documented, but not yet implemented, virtual amount scheme to solve this log::debug!("aggregating a chunk of {} inputs", N::MAX_INPUTS); plans.push(Plan { key: self.key, diff --git a/processor/src/networks/bitcoin.rs b/processor/src/networks/bitcoin.rs index 823b45ce..83b35f86 100644 --- a/processor/src/networks/bitcoin.rs +++ b/processor/src/networks/bitcoin.rs @@ -47,8 +47,8 @@ use crate::{ networks::{ NetworkError, Block as BlockTrait, OutputType, Output as OutputTrait, Transaction as TransactionTrait, SignableTransaction as SignableTransactionTrait, - Eventuality as EventualityTrait, EventualitiesTracker, PostFeeBranch, Network, drop_branches, - amortize_fee, + Eventuality as EventualityTrait, EventualitiesTracker, AmortizeFeeRes, PreparedSend, Network, + drop_branches, amortize_fee, }, Plan, }; @@ -509,8 +509,8 @@ impl Network for Bitcoin { _: usize, mut plan: Plan, fee: Fee, - ) -> Result<(Option<(SignableTransaction, Self::Eventuality)>, Vec), NetworkError> - { + operating_costs: u64, + ) -> Result, NetworkError> { let signable = |plan: &Plan, tx_fee: Option<_>| { let mut payments = vec![]; for payment in &plan.payments { @@ -558,23 +558,37 @@ impl Network for Bitcoin { let tx_fee = match signable(&plan, None) { Some(tx) => tx.needed_fee(), - None => return Ok((None, drop_branches(&plan))), + None => { + return Ok(PreparedSend { + tx: None, + post_fee_branches: drop_branches(&plan), + // We expected a change output of sum(inputs) - sum(outputs) + // Since we can no longer create this change output, it becomes an operating cost + operating_costs: operating_costs + + plan.inputs.iter().map(|input| input.amount()).sum::() - + plan.payments.iter().map(|payment| payment.amount).sum::(), + }); + } }; - let branch_outputs = amortize_fee(&mut plan, tx_fee); + let AmortizeFeeRes { post_fee_branches, operating_costs } = + amortize_fee(&mut plan, operating_costs, tx_fee); let signable = signable(&plan, Some(tx_fee)).unwrap(); + // TODO: If the change output was dropped by Bitcoin, increase operating costs + let plan_binding_input = *plan.inputs[0].output.outpoint(); let outputs = signable.outputs().to_vec(); - Ok(( - Some(( + Ok(PreparedSend { + tx: Some(( SignableTransaction { transcript: plan.transcript(), actual: signable }, Eventuality { plan_binding_input, outputs }, )), - branch_outputs, - )) + post_fee_branches, + operating_costs, + }) } async fn attempt_send( diff --git a/processor/src/networks/mod.rs b/processor/src/networks/mod.rs index 113cebb0..46b65c6e 100644 --- a/processor/src/networks/mod.rs +++ b/processor/src/networks/mod.rs @@ -209,19 +209,47 @@ pub fn drop_branches(plan: &Plan) -> Vec { branch_outputs } +pub struct AmortizeFeeRes { + post_fee_branches: Vec, + operating_costs: u64, +} + // Amortize a fee over the plan's payments -pub fn amortize_fee(plan: &mut Plan, tx_fee: u64) -> Vec { - // No payments to amortize over - if plan.payments.is_empty() { - return vec![]; - } +pub fn amortize_fee( + plan: &mut Plan, + operating_costs: u64, + tx_fee: u64, +) -> AmortizeFeeRes { + let total_fee = { + let mut total_fee = tx_fee; + // Since we're creating a change output, letting us recoup coins, amortize the operating costs + // as well + if plan.change.is_some() { + total_fee += operating_costs; + } + total_fee + }; let original_outputs = plan.payments.iter().map(|payment| payment.amount).sum::(); + // If this isn't enough for the total fee, drop and move on + if original_outputs < total_fee { + let mut remaining_operating_costs = operating_costs; + if plan.change.is_some() { + // Operating costs increase by the TX fee + remaining_operating_costs += tx_fee; + // Yet decrease by the payments we managed to drop + remaining_operating_costs = remaining_operating_costs.saturating_sub(original_outputs); + } + return AmortizeFeeRes { + post_fee_branches: drop_branches(plan), + operating_costs: remaining_operating_costs, + }; + } // Amortize the transaction fee across outputs let mut payments_len = u64::try_from(plan.payments.len()).unwrap(); // Use a formula which will round up - let per_output_fee = |payments| (tx_fee + (payments - 1)) / payments; + let per_output_fee = |payments| (total_fee + (payments - 1)) / payments; let post_fee = |payment: &Payment, per_output_fee| { let mut post_fee = payment.amount.checked_sub(per_output_fee); @@ -266,9 +294,26 @@ pub fn amortize_fee(plan: &mut Plan, tx_fee: u64) -> Vec(); - assert!((new_outputs + tx_fee) <= original_outputs); + assert!((new_outputs + total_fee) <= original_outputs); - branch_outputs + AmortizeFeeRes { + post_fee_branches: branch_outputs, + operating_costs: if plan.change.is_none() { + // If the change is None, this had no effect on the operating costs + operating_costs + } else { + // Since the change is some, and we successfully amortized, the operating costs were recouped + 0 + }, + } +} + +pub struct PreparedSend { + /// None for the transaction if the SignableTransaction was dropped due to lack of value. + pub tx: Option<(N::SignableTransaction, N::Eventuality)>, + pub post_fee_branches: Vec, + /// The updated operating costs after preparing this transaction. + pub operating_costs: u64, } #[async_trait] @@ -364,18 +409,15 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug { ) -> HashMap<[u8; 32], (usize, Self::Transaction)>; /// Prepare a SignableTransaction for a transaction. - /// - /// Returns None for the transaction if the SignableTransaction was dropped due to lack of value. - #[rustfmt::skip] + // TODO: These have common code inside them + // Provide prepare_send, have coins offers prepare_send_inner async fn prepare_send( &self, block_number: usize, plan: Plan, - fee: Self::Fee, - ) -> Result< - (Option<(Self::SignableTransaction, Self::Eventuality)>, Vec), - NetworkError - >; + fee_rate: Self::Fee, + running_operating_costs: u64, + ) -> Result, NetworkError>; /// Attempt to sign a SignableTransaction. async fn attempt_send( diff --git a/processor/src/networks/monero.rs b/processor/src/networks/monero.rs index 71d0027e..8958abf4 100644 --- a/processor/src/networks/monero.rs +++ b/processor/src/networks/monero.rs @@ -38,8 +38,8 @@ use crate::{ networks::{ NetworkError, Block as BlockTrait, OutputType, Output as OutputTrait, Transaction as TransactionTrait, SignableTransaction as SignableTransactionTrait, - Eventuality as EventualityTrait, EventualitiesTracker, PostFeeBranch, Network, drop_branches, - amortize_fee, + Eventuality as EventualityTrait, EventualitiesTracker, AmortizeFeeRes, PreparedSend, Network, + drop_branches, amortize_fee, }, }; @@ -399,7 +399,8 @@ impl Network for Monero { block_number: usize, mut plan: Plan, fee: Fee, - ) -> Result<(Option<(SignableTransaction, Eventuality)>, Vec), NetworkError> { + operating_costs: u64, + ) -> Result, NetworkError> { // Sanity check this has at least one output planned assert!((!plan.payments.is_empty()) || plan.change.is_some()); @@ -522,20 +523,28 @@ impl Network for Monero { let tx_fee = match signable(plan.clone(), None)? { Some(tx) => tx.fee(), - None => return Ok((None, drop_branches(&plan))), + None => { + return Ok(PreparedSend { + tx: None, + post_fee_branches: drop_branches(&plan), + // We expected a change output of sum(inputs) - sum(outputs) + // Since we can no longer create this change output, it becomes an operating cost + operating_costs: operating_costs + + plan.inputs.iter().map(|input| input.amount()).sum::() - + plan.payments.iter().map(|payment| payment.amount).sum::(), + }); + } }; - let branch_outputs = amortize_fee(&mut plan, tx_fee); + let AmortizeFeeRes { post_fee_branches, operating_costs } = + amortize_fee(&mut plan, operating_costs, tx_fee); - let signable = SignableTransaction { - transcript, - actual: match signable(plan, Some(tx_fee))? { - Some(signable) => signable, - None => return Ok((None, branch_outputs)), - }, - }; + // TODO: If the change output was dropped by Monero, increase operating costs + + let signable = + SignableTransaction { transcript, actual: signable(plan, Some(tx_fee))?.unwrap() }; let eventuality = signable.actual.eventuality().unwrap(); - Ok((Some((signable, eventuality)), branch_outputs)) + Ok(PreparedSend { tx: Some((signable, eventuality)), post_fee_branches, operating_costs }) } async fn attempt_send( diff --git a/processor/src/plan.rs b/processor/src/plan.rs index 3f005865..7cea2d07 100644 --- a/processor/src/plan.rs +++ b/processor/src/plan.rs @@ -74,7 +74,19 @@ impl Payment { pub struct Plan { pub key: ::G, pub inputs: Vec, + /// The payments this Plan is inteded to create. + /// + /// This should only contain payments leaving Serai. While it is acceptable for users to enter + /// Serai's address(es) as the payment address, as that'll be handled by anything which expects + /// certain properties, Serai as a system MUST NOT use payments for internal transfers. Doing + /// so will cause a reduction in their value by the TX fee/operating costs, creating an + /// incomplete transfer. pub payments: Vec>, + /// The change this Plan should use. + /// + /// This MUST contain a Serai address. Operating costs may be deducted from the payments in this + /// Plan on the premise that the change address is Serai's, and accordingly, Serai will recoup + /// the operating costs. pub change: Option, } impl core::fmt::Debug for Plan { diff --git a/processor/src/tests/addresses.rs b/processor/src/tests/addresses.rs index 9d726e03..44b470a6 100644 --- a/processor/src/tests/addresses.rs +++ b/processor/src/tests/addresses.rs @@ -42,10 +42,11 @@ async fn spend( change: Some(N::change_address(key)), }, network.get_fee().await, + 0, ) .await .unwrap() - .0 + .tx .unwrap(), ), ); diff --git a/processor/src/tests/signer.rs b/processor/src/tests/signer.rs index c5a0e6c5..88c4b5e3 100644 --- a/processor/src/tests/signer.rs +++ b/processor/src/tests/signer.rs @@ -171,10 +171,11 @@ pub async fn test_signer(network: N) { change: Some(N::change_address(key)), }, fee, + 0, ) .await .unwrap() - .0 + .tx .unwrap(); eventualities.push(eventuality.clone()); diff --git a/processor/src/tests/wallet.rs b/processor/src/tests/wallet.rs index 8bb84d45..aaf777b1 100644 --- a/processor/src/tests/wallet.rs +++ b/processor/src/tests/wallet.rs @@ -96,10 +96,10 @@ pub async fn test_wallet(network: N) { 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) + .prepare_send(network.get_block_number(&block_id).await, plans[0].clone(), fee, 0) .await .unwrap() - .0 + .tx .unwrap(); eventualities.push(eventuality.clone());