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

Paying to each holder in each group #649

Closed
wants to merge 29 commits into from

Conversation

joshuef
Copy link
Contributor

@joshuef joshuef commented Aug 16, 2023

starting to simplify payments to a per chunk basis

A first step in #638## Description

Summary generated by Reviewpad on 16 Aug 23 14:29 UTC

This pull request includes the following changes:

  1. In the lib.rs file:
    • Import PublicKey from libp2p::identity and use it in addition to Multiaddr from libp2p.
    • Add a new field reward_main_key of type PublicKey to the struct Node.
  2. In the file_apis.rs file:
    • Import statement added: use crate::NetworkAddress.
    • Import statement removed: use crate::messages::PaymentProof.
    • Import statement added: use sn_dbc::DbcId.
    • Method added: network_address(), returns the NetworkAddress associated with the chunk.
    • Struct field type changed: payment changed from PaymentProof to Vec<DbcId>.
  3. In the api.rs file:
    • Added PublicKey import from identity module.
    • Renamed method get_store_cost_from_network to get_store_costs_from_network and changed its return type to Result<Vec<(PublicKey, Token)>>.
    • Updated error handling and added decoding of PublicKey in get_store_costs_from_network method.
    • Added a retry loop for put_record_with_retries method.
    • Made get_closest_peers method public.
  4. In the storage_payments.rs file:
    • Import sn_dbc::{Hash, Token} has been removed.
    • The type of random_content_addrs has been changed from XorName to sn_protocol::NetworkAddress::ChunkAddress.
    • The eyre::Result import has been simplified.
    • Various changes related to the variable invalid_proofs have been made in the function storage_payment_chunk_upload_fails.
  5. In the mod.rs file in the sn_protocol/src/messages directory:
    • In the use statement, PaymentProof has been replaced with PaymentTransactions.
  6. In the data_with_churn.rs file:
    • The method pay_for_storage is modified to use the network_address() method instead of the name() method on each element of the chunks iterator.
  7. In the verify_data_location.rs file:
    • There is a change in the store_chunk function where the pay_for_storage method is called with the network_address() method instead of the name() method on each element of the chunks iterator.
  8. In the sn_cli/src/subcommands/files.rs file:
    • The import statement sn_transfers::wallet::PaymentProofsMap has been replaced with sn_transfers::wallet::PaymentTransactionsMap.
    • The argument payment_proofs in the function upload_chunks has been changed from &PaymentProofsMap to &PaymentTransactionsMap.
  9. In the wallet.rs file:
    • The type PaymentProofsMap has been renamed to PaymentTransactionsMap.
    • The get_payment_proof method now accepts a NetworkAddress parameter instead of XorName, and returns an Option<&Vec<DbcId>> instead of Option<&PaymentProof>.
    • The add_payment_proofs method now accepts a PaymentTransactionsMap parameter instead of PaymentProofsMap.
    • The local_send_storage_payment method now accepts a storage_cost_for_addr parameter of type Token instead of storage_payment and root_hash.
    • The wallet.paymet_proofs field has been renamed to wallet.payment_transactions.
  10. In the .github/workflows/memcheck.yml file:
    • Line 101: The timeout-minutes value has been updated from 25 to 40.
  11. In the sn_networking/src/error.rs file:
    • Added use libp2p::identity::DecodingError;.
    • Added variant Libp2pDecode to the Error enum with a DecodingError parameter.
  12. In the sn_protocol/src/messages/cmd.rs file:
    • The struct PaymentProof has been renamed to PaymentTransactions.
    • The fields audit_trail and path of the PaymentTransactions struct have been removed.
  13. In the sn_client module:
    • The network_store_cost field has been removed from the Client struct.
  14. In the transfer.rs file:
    • Line 66: The root_hash variable is no longer passed as a parameter to the function create_storage_payment_transfer().
    • Line 70: Added a comment to indicate that some code needs to be cleaned up in the future.
    • Line 72: The root_hash variable is now created using the sn_dbc::Hash::hash() function and the value "nonsense" as input.
  15. In the sn_protocol/src/messages/mod.rs file:
    • The import statement for sn_protocol::storage::ChunkAddress has been removed.

Please review these changes and let me know if you have any questions or need further assistance.

@reviewpad reviewpad bot added the Large Large sized PR label Aug 16, 2023
sn_transfers/src/client_transfers/transfer.rs Fixed Show fixed Hide fixed
sn_node/src/put_validation.rs Fixed Show fixed Hide fixed
sn_client/src/wallet.rs Fixed Show fixed Hide fixed
@@ -776,6 +781,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.

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
sn_networking/src/lib.rs Fixed Show fixed Hide fixed
@joshuef joshuef force-pushed the PayingEachHolder2 branch 3 times, most recently from e8bc53c to ab1b712 Compare August 17, 2023 12:01
sn_transfers/src/client_transfers/transfer.rs Fixed Show fixed Hide fixed
sn_node/src/api.rs Dismissed Show dismissed Hide dismissed
sn_node/src/put_validation.rs Fixed Show fixed Hide fixed
sn_node/src/put_validation.rs Fixed Show fixed Hide fixed
sn_node/src/put_validation.rs Fixed Show fixed Hide fixed
sn_client/src/wallet.rs Fixed Show fixed Hide fixed
sn_client/src/wallet.rs Fixed Show fixed Hide fixed
sn_client/src/wallet.rs Fixed Show fixed Hide fixed
sn_networking/src/lib.rs Fixed Show fixed Hide fixed
@joshuef joshuef force-pushed the PayingEachHolder2 branch 2 times, most recently from c786fbd to c2eb1f3 Compare August 17, 2023 12:08
@@ -76,5 +83,6 @@
#[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
pub struct ChunkWithPayment {
pub chunk: Chunk,
pub payment: PaymentProof,
/// TODO: encrypt this or supply only encrypted derivation index

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
addr_name,
reason: err.to_string(),
})?;
// TODO: properly verify which one of these was for this node, and check _that_

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
assert!(matches!(
verify_fee_output_and_proof(name, network_store_cost, &tx, audit_trail, &[]),
Err(err) if err == ProtocolError::PaymentProofInvalidFeeOutput(invalid_fee_id)));
// TODO: sort out what is acceptable based on the network store cost

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
sn_node/src/put_validation.rs Fixed Show fixed Hide fixed
inputs: vec![],
outputs: vec![],
outputs: tx_payments.clone(),
// TODO: Clean this up when we remove FeeOutput

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
@joshuef joshuef marked this pull request as ready for review August 17, 2023 21:39
@reviewpad reviewpad bot requested a review from bochaco August 17, 2023 21:40
@reviewpad
Copy link

reviewpad bot commented Aug 17, 2023

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' (a0b000c)

  • Unconventional commit detected: 'feat!(protocol): get price and pay for each chunk individually' (913c81d)
  • Unconventional commit detected: 'feat!(protocol): gets keys with GetStoreCost' (deaa859)
  • Unconventional title detected: 'Paying to each holder in each group' illegal 'a' character in commit message type: col=01

@joshuef joshuef force-pushed the PayingEachHolder2 branch 2 times, most recently from 6e3a661 to 30374eb Compare August 18, 2023 06:58
@joshuef joshuef mentioned this pull request Aug 18, 2023
joshuef and others added 20 commits August 25, 2023 13:55
Sets us up to pay every node that will be storing the data.
We GetStoreCost and pay to their key, and put the chunk with the DBCs

This is not yet validated at the node side
This gives maximal resilience as a place to start.
(This is required for data location verification tests eg)
joshuef and others added 3 commits August 25, 2023 17:37
Holding DBCs causes large mem spikes. For now we allow for this on CI/
We'll refactor this away down the line
@joshuef joshuef closed this Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Large Large sized PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants