Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Massive Spend Refactor #1989

Merged
merged 28 commits into from
Aug 28, 2024
Merged
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
abc3df2
chore!: refactor spend struct
maqi Jun 29, 2024
75afa35
fix(spend): support multiple inputs with different keys
maqi Jul 9, 2024
b66d5a9
chore(kad): not enough close peers no longer result in error out
maqi Jul 10, 2024
cbde3dc
chore(spend_simulation): disable non-cashnote UTXO spend error out
maqi Jul 10, 2024
9942c03
chore(spend_simulation): disable failing query attempt assertion
maqi Jul 10, 2024
b187c8f
feat: add support for multi parents in CNR
grumbach Jul 11, 2024
8bb3a29
feat: remove middle spend from offline transfers
grumbach Jul 12, 2024
56a56cf
chore: rename token to amount
grumbach Jul 18, 2024
2c1ca64
refactor: entire transacting flow from CashNote to SignedSpend
grumbach Jul 19, 2024
9a54e12
fix: move tx verification to wallet sign
grumbach Jul 19, 2024
17d95d2
fix: re enable reissue bench
grumbach Jul 22, 2024
178b01c
fix: doctests
grumbach Jul 22, 2024
cabb759
fix: bench
grumbach Jul 22, 2024
e48af65
feat: improve network royalties identification for spends
grumbach Jul 22, 2024
dd91c1c
fix: spends fixes and security verifications
grumbach Jul 23, 2024
386d1c7
chore: improve code quality
grumbach Jul 23, 2024
9bd610a
fix: dag double spend test
grumbach Jul 23, 2024
d3c9e39
fix: serious error in spend dag double spend branching
grumbach Jul 23, 2024
fb33e07
fix: dag crawling invalid source check
grumbach Jul 23, 2024
fbc6613
fix: re comment uncommented block
grumbach Jul 23, 2024
dbe8822
chore: update docs for CashNote and spends
grumbach Jul 24, 2024
0f92260
feat: fixes and tests for unsigned tx
grumbach Jul 25, 2024
f574fb5
chore: disable flawed tests sending out invalid 0 tx
grumbach Jul 25, 2024
c2b3bb1
fix: allow time for double spend to spread in spend test
grumbach Jul 29, 2024
9fe6f44
chore: adapt after rebase
grumbach Jul 29, 2024
8bcab6d
test: bump sleep time to 20s
grumbach Jul 29, 2024
bab2054
fix: test issue
grumbach Jul 29, 2024
4643330
feat: remove output purpose
grumbach Aug 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: remove middle spend from offline transfers
grumbach committed Jul 29, 2024
commit 8bb3a291213dbad1d81b905c11223c6cfdd8ba13
1 change: 0 additions & 1 deletion sn_client/src/audit/tests/setup.rs
Original file line number Diff line number Diff line change
@@ -114,7 +114,6 @@ impl MockNetwork {
recipient,
from_wallet.sk.main_pubkey(),
SpendReason::default(),
None,
)
.map_err(|e| eyre!("failed to create transfer: {}", e))?;
let spends = transfer.all_spend_requests;
40 changes: 8 additions & 32 deletions sn_node/tests/double_spend.rs
Original file line number Diff line number Diff line change
@@ -54,15 +54,9 @@ async fn cash_note_transfer_double_spend_fail() -> Result<()> {
let to2_unique_key = (amount, to2, DerivationIndex::random(&mut rng));
let to3_unique_key = (amount, to3, DerivationIndex::random(&mut rng));

let transfer_to_2 = OfflineTransfer::new(
some_cash_notes,
vec![to2_unique_key],
to1,
reason.clone(),
None,
)?;
let transfer_to_3 =
OfflineTransfer::new(same_cash_notes, vec![to3_unique_key], to1, reason, None)?;
let transfer_to_2 =
OfflineTransfer::new(some_cash_notes, vec![to2_unique_key], to1, reason.clone())?;
let transfer_to_3 = OfflineTransfer::new(same_cash_notes, vec![to3_unique_key], to1, reason)?;

// send both transfers to the network
// upload won't error out, only error out during verification.
@@ -127,8 +121,7 @@ async fn genesis_double_spend_fail() -> Result<()> {
);
let change_addr = second_wallet_addr;
let reason = SpendReason::default();
let transfer =
OfflineTransfer::new(genesis_cashnote, vec![recipient], change_addr, reason, None)?;
let transfer = OfflineTransfer::new(genesis_cashnote, vec![recipient], change_addr, reason)?;

// send the transfer to the network which will mark genesis as a double spent
// making its direct descendants unspendable
@@ -160,7 +153,6 @@ async fn genesis_double_spend_fail() -> Result<()> {
vec![recipient],
change_addr,
reason,
None,
)?;

// send the transfer to the network which should reject it
@@ -199,7 +191,6 @@ async fn poisoning_old_spend_should_not_affect_descendant() -> Result<()> {
vec![to_2_unique_key],
to1,
reason.clone(),
None,
)?;

info!("Sending 1->2 to the network...");
@@ -224,13 +215,8 @@ async fn poisoning_old_spend_should_not_affect_descendant() -> Result<()> {
wallet_22.address(),
DerivationIndex::random(&mut rng),
);
let transfer_to_22 = OfflineTransfer::new(
cash_notes_2,
vec![to_22_unique_key],
to2,
reason.clone(),
None,
)?;
let transfer_to_22 =
OfflineTransfer::new(cash_notes_2, vec![to_22_unique_key], to2, reason.clone())?;

client
.send_spends(transfer_to_22.all_spend_requests.iter(), false)
@@ -251,13 +237,8 @@ async fn poisoning_old_spend_should_not_affect_descendant() -> Result<()> {
wallet_3.address(),
DerivationIndex::random(&mut rng),
);
let transfer_to_3 = OfflineTransfer::new(
cash_notes_1,
vec![to_3_unique_key],
to1,
reason.clone(),
None,
)?; // reuse the old cash notes
let transfer_to_3 =
OfflineTransfer::new(cash_notes_1, vec![to_3_unique_key], to1, reason.clone())?; // reuse the old cash notes
client
.send_spends(transfer_to_3.all_spend_requests.iter(), false)
.await?;
@@ -290,7 +271,6 @@ async fn poisoning_old_spend_should_not_affect_descendant() -> Result<()> {
vec![to_222_unique_key],
wallet_22.address(),
reason,
None,
)?;
client
.send_spends(transfer_to_222.all_spend_requests.iter(), false)
@@ -358,7 +338,6 @@ async fn parent_and_child_double_spends_should_lead_to_cashnote_being_invalid()
vec![to_b_unique_key],
wallet_a.address(),
reason.clone(),
None,
)?;

info!("Sending A->B to the network...");
@@ -388,7 +367,6 @@ async fn parent_and_child_double_spends_should_lead_to_cashnote_being_invalid()
vec![to_c_unique_key],
wallet_b.address(),
reason.clone(),
None,
)?;

info!("spend B to C: {:?}", transfer_to_c.all_spend_requests);
@@ -416,7 +394,6 @@ async fn parent_and_child_double_spends_should_lead_to_cashnote_being_invalid()
vec![to_x_unique_key],
wallet_a.address(),
reason.clone(),
None,
)?; // reuse the old cash notes
client
.send_spends(transfer_to_x.all_spend_requests.iter(), false)
@@ -447,7 +424,6 @@ async fn parent_and_child_double_spends_should_lead_to_cashnote_being_invalid()
vec![to_y_unique_key],
wallet_b.address(),
reason.clone(),
None,
)?; // reuse the old cash notes

info!("spend B to Y: {:?}", transfer_to_y.all_spend_requests);
9 changes: 0 additions & 9 deletions sn_node/tests/spend_simulation.rs
Original file line number Diff line number Diff line change
@@ -357,18 +357,11 @@ async fn inner_handle_action(
"{our_id} Available CashNotes for local send: {:?}",
available_cash_notes
);
let mut rng = &mut rand::rngs::OsRng;
let derivation_index = DerivationIndex::random(&mut rng);
let transfer = OfflineTransfer::new(
available_cash_notes,
recipients,
wallet.address(),
SpendReason::default(),
Some((
wallet.key().main_pubkey(),
derivation_index,
wallet.key().derive_key(&derivation_index),
)),
)?;
let recipient_cash_notes = transfer.cash_notes_for_recipient.clone();
let change = transfer.change_cash_note.clone();
@@ -414,7 +407,6 @@ async fn inner_handle_action(
vec![to],
wallet.address(),
SpendReason::default(),
None,
)?;
info!("{our_id} double spending transfer: {transfer:?}");

@@ -703,7 +695,6 @@ async fn init_state(count: usize) -> Result<(Client, State)> {
recipients,
first_wallet.address(),
reason.clone(),
None,
)?;

info!("Sending transfer for all wallets and verifying them");
8 changes: 0 additions & 8 deletions sn_transfers/benches/reissue.rs
Original file line number Diff line number Diff line change
@@ -41,7 +41,6 @@ fn bench_reissue_1_to_100(c: &mut Criterion) {
recipients,
starting_main_key.main_pubkey(),
SpendReason::default(),
None,
)
.expect("transfer to succeed");

@@ -98,7 +97,6 @@ fn bench_reissue_100_to_1(c: &mut Criterion) {
recipients,
starting_main_key.main_pubkey(),
SpendReason::default(),
None,
)
.expect("transfer to succeed");

@@ -136,18 +134,12 @@ fn bench_reissue_100_to_1(c: &mut Criterion) {
DerivationIndex::random(&mut rng),
)];

let derivation_index = DerivationIndex::random(&mut rng);
// create transfer to merge all of the cashnotes into one
let many_to_one_transfer = OfflineTransfer::new(
many_cashnotes,
one_single_recipient,
starting_main_key.main_pubkey(),
SpendReason::default(),
Some((
starting_main_key.main_pubkey(),
derivation_index,
starting_main_key.derive_key(&derivation_index),
)),
)
.expect("transfer to succeed");

104 changes: 3 additions & 101 deletions sn_transfers/src/transfers/offline_transfer.rs
Original file line number Diff line number Diff line change
@@ -108,16 +108,11 @@ impl OfflineTransfer {
/// The peers will validate each signed spend they receive, before accepting it.
/// Once enough peers have accepted all the spends of the transaction, and serve
/// them upon request, the transaction will be completed.
///
/// When there are multiple inputs from different unique_pubkeys,
/// they shall all be paid into a `middle_addr` first, then from that `middle_addr` pay out
/// to recipients as normal.
pub fn new(
available_cash_notes: CashNotesAndSecretKey,
recipients: Vec<(NanoTokens, MainPubkey, DerivationIndex)>,
change_to: MainPubkey,
input_reason_hash: SpendReason,
middle_addr: Option<(MainPubkey, DerivationIndex, DerivedSecretKey)>,
) -> Result<Self> {
let total_output_amount = recipients
.iter()
@@ -140,7 +135,7 @@ impl OfflineTransfer {
change: (change_amount, change_to),
};

create_offline_transfer_with(selected_inputs, input_reason_hash, middle_addr)
create_offline_transfer_with(selected_inputs, input_reason_hash)
}

pub fn verify(&self, main_key: &MainSecretKey) -> Result<()> {
@@ -315,103 +310,9 @@ fn create_transaction_builder_with(
/// to the network. When those same signed spends can be retrieved from
/// enough peers in the network, the transaction will be completed.
fn create_offline_transfer_with(
mut selected_inputs: TransferInputs,
selected_inputs: TransferInputs,
input_reason: SpendReason,
middle_addr: Option<(MainPubkey, DerivationIndex, DerivedSecretKey)>,
) -> Result<OfflineTransfer> {
let mut all_spend_requests = vec![];

let input_keys: BTreeSet<_> = selected_inputs
.cash_notes_to_spend
.iter()
.map(|(cn, _)| cn.unique_pubkey)
.collect();
if input_keys.len() > 1 {
info!(
"Multiple input_keys vs multiple outputs detected {:?}",
input_keys
);
if let Some((main_pubkey, derivation_index, middle_derived_sk)) = middle_addr {
let mut middle_cash_notes = vec![];
for input_key in input_keys {
let mut amount: u64 = 0;
let cash_notes_to_spend = selected_inputs
.cash_notes_to_spend
.iter()
.filter_map(|(cn, derived_sk)| {
if cn.unique_pubkey == input_key {
if let Ok(value) = cn.value() {
amount += value.as_nano();
Some((cn.clone(), derived_sk.clone()))
} else {
None
}
} else {
None
}
})
.collect();

let recipients = vec![(NanoTokens::from(amount), main_pubkey, derivation_index)];

let middle_inputs = TransferInputs {
cash_notes_to_spend,
recipients,
change: (NanoTokens::zero(), selected_inputs.change.1),
};

let (tx_builder, _change_id) = create_transaction_builder_with(middle_inputs)?;
let cash_note_builder = tx_builder.build(input_reason.clone());
let signed_spends: BTreeMap<_, _> = cash_note_builder
.signed_spends()
.into_iter()
.map(|spend| (spend.unique_pubkey(), spend))
.collect();

for (_, signed_spend) in signed_spends.into_iter() {
all_spend_requests.push(signed_spend.to_owned());
}
middle_cash_notes.extend(
cash_note_builder
.build()?
.into_iter()
.map(|(cash_note, _)| cash_note)
.collect::<Vec<_>>(),
);
}

info!("We now have middle cash notes: {middle_cash_notes:?}");

let mut parent_spends = BTreeSet::new();
for cn in middle_cash_notes.iter() {
for parent_spend in cn.parent_spends.iter() {
let _ = parent_spends.insert(parent_spend.clone());
}
}

let cash_notes_to_spend = if let Some(cn) = middle_cash_notes.first() {
let merged_cn = CashNote {
unique_pubkey: cn.unique_pubkey,
parent_spends,
main_pubkey: cn.main_pubkey,
derivation_index: cn.derivation_index,
};
info!("We now have a merged cash note: {merged_cn:?}");
vec![(merged_cn, Some(middle_derived_sk.clone()))]
} else {
return Err(TransferError::MultipleInputsWithoutMiddlePayment);
};

selected_inputs = TransferInputs {
cash_notes_to_spend,
recipients: selected_inputs.recipients,
change: selected_inputs.change,
};
} else {
return Err(TransferError::MultipleInputsWithoutMiddleAddr);
}
}

let (tx_builder, change_id) = create_transaction_builder_with(selected_inputs)?;

// Finalize the tx builder to get the cash_note builder.
@@ -423,6 +324,7 @@ fn create_offline_transfer_with(
.map(|spend| (spend.unique_pubkey(), spend))
.collect();

let mut all_spend_requests = vec![];
for (_, signed_spend) in signed_spends.into_iter() {
all_spend_requests.push(signed_spend.to_owned());
}
26 changes: 2 additions & 24 deletions sn_transfers/src/wallet/hot_wallet.rs
Original file line number Diff line number Diff line change
@@ -424,18 +424,8 @@ impl HotWallet {

let reason = reason.unwrap_or_default();

let derivation_index = DerivationIndex::random(&mut rng);
let transfer = OfflineTransfer::new(
available_cash_notes,
to_unique_keys,
self.address(),
reason,
Some((
self.key.main_pubkey(),
derivation_index,
self.key.derive_key(&derivation_index),
)),
)?;
let transfer =
OfflineTransfer::new(available_cash_notes, to_unique_keys, self.address(), reason)?;

let created_cash_notes = transfer.cash_notes_for_recipient.clone();

@@ -498,18 +488,12 @@ impl HotWallet {
.into_iter()
.map(|(amount, address)| (amount, address, DerivationIndex::random(&mut rng)))
.collect();
let derivation_index = DerivationIndex::random(&mut rng);

let transfer = OfflineTransfer::new(
available_cash_notes,
to_unique_keys,
self.address(),
spend_reason,
Some((
self.key.main_pubkey(),
derivation_index,
self.key.derive_key(&derivation_index),
)),
)?;

let signed_spends = transfer.all_spend_requests.clone();
@@ -588,17 +572,11 @@ impl HotWallet {

let spend_reason = Default::default();
let start = Instant::now();
let derivation_index = DerivationIndex::random(&mut rng);
let offline_transfer = OfflineTransfer::new(
available_cash_notes,
recipients,
self.address(),
spend_reason,
Some((
self.key.main_pubkey(),
derivation_index,
self.key.derive_key(&derivation_index),
)),
)?;
trace!(
"local_send_storage_payment created offline_transfer with {} cashnotes in {:?}",