Pay per chunk #647

joshuef · 2023-08-16T09:51:16Z

starting to simplify payments to a per chunk basis

A first step in #638## Description

Summary generated by Reviewpad on 16 Aug 23 09:51 UTC

This pull request includes multiple file diffs with various changes, such as renaming types and fields, importing and removing modules, updating function signatures and return types, and modifying method implementations. These changes seem to be focused on improving code clarity, making updates for better functionality, and enhancing the payment transaction handling.

Please review the changes thoroughly to ensure they align with the intended goals and do not introduce any unintended issues or conflicts. Let me know if you need further details or assistance with the code review.

maqi · 2023-08-16T10:05:16Z

sn_client/src/wallet.rs

+
+        trace!("Making payment for of {amount_to_pay:?} nano tokens to each node in close group at {content_addr:?}.");
+
+        // TODO: This needs to go out to each CLOSEGROUP of addresses


you mean paying each individual holder ?

hmm... I don't think we need to contact each holder individually,
just get_closest(chunk_addr) then transfer some amount to each fetched close_peer, and send the transfer to network shall be enough?

just, that means the payment_proof needs to hold 8 spend_ids, and on validation, the holder only needs to validate the spend to itself (or still ALL for security?)

Each CLOSE_GROUP for each record address needs to return a price, which should then be paid.

on validation, each would just do itself, I think?

Each CLOSE_GROUP for each record address needs to return a price, which should then be paid.

ok. I thought a close_group can charge with the same.

on validation, each would just do itself, I think?

yeah. but that's can be done on receiving the record via put_record , instead of contacting them directly individually.

sn_transfers/src/client_transfers/transfer.rs

    reason_hash: Hash,
 ) -> Result<TransferOutputs> {
    // We need to select the necessary number of dbcs from those that we were passed.
    let (dbcs_to_spend, change_amount) = select_inputs(available_dbcs, storage_payment)?;

+    // TODO: Clear all this up when we've removed this


sn_node/src/put_validation.rs

-                        panic!("Expected insufficient amount error")
-                    }
-                }
+                // TODO: Add a check on cost when we're paying DBCs


sn_client/src/wallet.rs

-            self.store_cost().as_nano() * (2.0f64.powf((num_of_addrs / 100 + 1) as f64)) as u64,
-        );
+        for content_addr in addrs_to_pay {
+            // TODO: parallelise this


maqi · 2023-08-16T09:56:07Z

sn_client/src/api.rs

@@ -292,7 +275,7 @@ impl Client {
    pub(super) async fn store_chunk(
        &self,
        chunk: Chunk,
-        payment: PaymentProof,
+        payment: Vec<DbcId>,


I thought there is only one DbcId of the payment for one chunk ?

I think you may need to reissue several DBCs sometimes if you have a lot of dust?

sn_client/src/wallet.rs

reviewpad · 2023-08-16T10:14:44Z

Reviewpad Report

‼️ Errors

Unconventional commit detected: 'feat!(protocol): remove chunk merkletree to simplify payment

starting to simplify payments to a per chunk basis

A first step in #638' (53a1653)

Unconventional commit detected: 'feat!(protocol): get price and pay for each chunk individually' (bc38914)
Unconventional title detected: 'Pay per chunk' illegal 'a' character in commit message type: col=01

joshuef · 2023-08-16T10:24:53Z

This closes #638

sn_networking/src/lib.rs

@@ -776,6 +775,8 @@
    /// Retry up to `PUT_RECORD_RETRIES` times if we can't verify the record is stored
    async fn put_record_with_retries(&self, record: Record) -> Result<()> {
        let mut retries = 0;
+
+        // TODO: Move this put retry loop up above store cost checks so we can re-put if storecost failed.


Previously we were hanging waiting on inactivity before we continued, now we push the loop forward and can us bootstrap to get us going

starting to simplify payments to a per chunk basis A first step in maidsafe#638

joshuef · 2023-08-18T06:58:57Z

Closing as #649 is atop this and working better

reviewpad bot added the Large Large sized PR label Aug 16, 2023

github-advanced-security bot found potential problems Aug 16, 2023

View reviewed changes

maqi reviewed Aug 16, 2023

View reviewed changes

joshuef force-pushed the PayingPerChunk branch from d7075cd to 082f18e Compare August 16, 2023 10:07

joshuef changed the title ~~feat!(protocol): remove chunk merkletree to simplify payment~~ Pay per chunk Aug 16, 2023

joshuef force-pushed the PayingPerChunk branch from 082f18e to f5c5409 Compare August 16, 2023 10:12

joshuef marked this pull request as ready for review August 16, 2023 10:14

reviewpad bot added the waiting-for-review label Aug 16, 2023

joshuef force-pushed the PayingPerChunk branch from f5c5409 to 3f5097c Compare August 16, 2023 10:24

maqi approved these changes Aug 16, 2023

View reviewed changes

joshuef force-pushed the PayingPerChunk branch from 3f5097c to 8f5132c Compare August 16, 2023 11:01

joshuef enabled auto-merge August 16, 2023 11:01

joshuef force-pushed the PayingPerChunk branch 2 times, most recently from 11a081a to 0e9297d Compare August 16, 2023 14:08

github-advanced-security bot found potential problems Aug 16, 2023

View reviewed changes

joshuef force-pushed the PayingPerChunk branch from 0e9297d to b565c26 Compare August 16, 2023 14:30

joshuef added 2 commits August 17, 2023 07:56

fix(client): use boostrap and fire Connecting event

331f5c4

Previously we were hanging waiting on inactivity before we continued, now we push the loop forward and can us bootstrap to get us going

fix(logging): get log name per bin

99911ba

joshuef force-pushed the PayingPerChunk branch from b565c26 to ee14b09 Compare August 17, 2023 07:20

joshuef added 3 commits August 17, 2023 08:30

ci: fix client log naming

6868792

feat!(protocol): remove chunk merkletree to simplify payment

53a1653

starting to simplify payments to a per chunk basis A first step in maidsafe#638

feat!(protocol): get price and pay for each chunk individually

bc38914

joshuef force-pushed the PayingPerChunk branch from ee14b09 to bc38914 Compare August 17, 2023 07:31

joshuef added this pull request to the merge queue Aug 17, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 17, 2023

joshuef added this pull request to the merge queue Aug 17, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 17, 2023

joshuef mentioned this pull request Aug 17, 2023

feat!(protocol): starting to simplify payments to a per chunk basis #646

Closed

joshuef closed this Aug 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pay per chunk #647

Pay per chunk #647

joshuef commented Aug 16, 2023 •

edited by reviewpad bot

maqi Aug 16, 2023

joshuef Aug 16, 2023

maqi Aug 16, 2023

maqi Aug 16, 2023

joshuef Aug 16, 2023

reviewpad bot commented Aug 16, 2023 •

edited

joshuef commented Aug 16, 2023

joshuef commented Aug 18, 2023


		trace!("Making payment for of {amount_to_pay:?} nano tokens to each node in close group at {content_addr:?}.");

		// TODO: This needs to go out to each CLOSEGROUP of addresses

Pay per chunk #647

Pay per chunk #647

Conversation

joshuef commented Aug 16, 2023 • edited by reviewpad bot

Summary generated by Reviewpad on 16 Aug 23 09:51 UTC

maqi Aug 16, 2023

Choose a reason for hiding this comment

joshuef Aug 16, 2023

Choose a reason for hiding this comment

maqi Aug 16, 2023

Choose a reason for hiding this comment

maqi Aug 16, 2023

Choose a reason for hiding this comment

joshuef Aug 16, 2023

Choose a reason for hiding this comment

reviewpad bot commented Aug 16, 2023 • edited

joshuef commented Aug 16, 2023

joshuef commented Aug 18, 2023

joshuef commented Aug 16, 2023 •

edited by reviewpad bot

reviewpad bot commented Aug 16, 2023 •

edited