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

feat: offline signing wallet cmds #1126

Merged
merged 8 commits into from
Jan 10, 2024

Conversation

bochaco
Copy link
Contributor

@bochaco bochaco commented Jan 3, 2024

Description

Summary generated by Reviewpad on 03 Jan 24 17:57 UTC

This pull request includes changes in multiple files. Here is a summary of the diff:

  • The local_store.rs file now returns a tuple containing a Vec of (CashNote, Option<DerivedSecretKey>)> instead of (Vec<(CashNote, DerivedSecretKey)>, WalletExclusiveAccess) in the available_cash_notes function. It also adds the build_unsigned_transaction function.
  • The reissue.rs file in the sn_transfers/benches directory has modifications in the create_offline_transfer and derive_key functions. The second element of the tuple parameter in the create_offline_transfer function is now wrapped in an Option type.
  • The offline_transfer.rs file includes changes in the TranferInputs struct, create_offline_transfer function, addition of build_unsigned_transaction function, modification of the select_inputs function, addition of create_transaction_builder_with function, and addition of the reason_hash parameter in the create_offline_transfer_with function.
  • The builder.rs file changes the type and parameter of input_details in TransactionBuilder, as well as the parameter types in the add_input and add_inputs methods.
  • The wallet.rs file introduces changes related to the Create and Transaction commands, along with the addition of the build_unsigned_transaction and receive helper functions.
  • The main.rs file adds a new arm in the match expression for the Transaction variant in the WalletCmds enum.
  • The mod.rs file introduces the build_unsigned_transaction function in the offline_transfer module.
  • The genesis.rs file modifies the create_first_cash_note_from_key method by adding an optional derived key as an argument in the add_input method.

Please let me know if you need any further assistance.

@reviewpad reviewpad bot added the Medium Medium sized PR label Jan 3, 2024
@bochaco bochaco force-pushed the feat-offline-wallet-cmds branch 2 times, most recently from 5639420 to 5c83a3a Compare January 3, 2024 18:49
@reviewpad reviewpad bot added Large Large sized PR and removed Medium Medium sized PR labels Jan 3, 2024
@bochaco bochaco force-pushed the feat-offline-wallet-cmds branch 6 times, most recently from 9e8e3e5 to 499b0e6 Compare January 5, 2024 19:15
sn_cli/src/subcommands/wallet.rs Fixed Show fixed Hide fixed
sn_cli/src/subcommands/wallet.rs Fixed Show fixed Hide fixed
@bochaco bochaco force-pushed the feat-offline-wallet-cmds branch from 499b0e6 to 8eed285 Compare January 5, 2024 19:17
@bochaco bochaco marked this pull request as ready for review January 5, 2024 19:18
@reviewpad reviewpad bot requested a review from joshuef January 5, 2024 19:18
@bochaco bochaco force-pushed the feat-offline-wallet-cmds branch from 8eed285 to 27f5976 Compare January 5, 2024 19:28
Comment on lines 108 to 110
Error::CashNoteReissueFailed(
"Overflow occurred while summing the amounts for the recipients.".to_string(),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make use of this error instead? Error::ExcessiveNanoValue? Makes it easily trackable.

@joshuef joshuef force-pushed the feat-offline-wallet-cmds branch from 27f5976 to 9dfa00d Compare January 9, 2024 15:21
match SecretKey::from_hex(key) {
Ok(sk) => {
let main_sk = MainSecretKey::new(sk);
// TODO: encrypt wallet file

Check notice

Code scanning / devskim

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

Suspicious comment
@joshuef joshuef enabled auto-merge January 9, 2024 15:24
@joshuef joshuef added this pull request to the merge queue Jan 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 9, 2024
@joshuef joshuef added this pull request to the merge queue Jan 10, 2024
Merged via the queue into maidsafe:main with commit 559fb83 Jan 10, 2024
30 checks passed
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.

4 participants