feat: watch-only wallet #1045
Merged
feat: watch-only wallet #1045
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bochaco
force-pushed
the
feat-watch-only-wallet
branch
from
31c32fe
to
25bd085
Compare
bochaco
force-pushed
the
feat-watch-only-wallet
branch
from
25bd085
to
2a9d000
Compare
bochaco
force-pushed
the
feat-watch-only-wallet
branch
from
2a9d000
to
9a94f10
Compare
bochaco
force-pushed
the
feat-watch-only-wallet
branch
from
9a94f10
to
9bd8b0f
Compare
grumbach
reviewed
Dec 6, 2023
Comment on lines
-170
to
+134
| let (key, wallet, unconfirmed_spend_requests) = load_from_path(wallet_dir, main_key)?; | ||
| Ok(Self { | ||
| key, | ||
| wallet, | ||
| wallet_dir: wallet_dir.to_path_buf(), | ||
| unconfirmed_spend_requests, | ||
| }) | ||
| Self::load_from_path_and_key(wallet_dir, main_key) |
grumbach
reviewed
Dec 6, 2023
Comment on lines
155
to
157
| pub fn balance(&self) -> NanoTokens { | ||
| self.wallet.balance() | ||
| self.watchonly_wallet.balance() | ||
| } |
There was a problem hiding this comment.
Any chance we can short circuit that one?
We have 4 functions named balance they call each other in a deep nested way that makes the code hard to navigate, any chance we can get rid of that LocalWallet middle man and directly call your new wallet?
There was a problem hiding this comment.
You are missing that the LocalWallet is a different type of wallet, it's like a hot-wallet (eventually we should rename it) which reuses the impl/features of a watch-only wallet. Nevertheless, a refactoring of the LocalWallet is out of the scope here.
grumbach
reviewed
Dec 6, 2023
Comment on lines
395
to
398
| pub fn deposit_and_store_to_disk(&mut self, received_cash_notes: &Vec<CashNote>) -> Result<()> { | ||
| if received_cash_notes.is_empty() { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| // lock and load from disk to make sure we're up to date and others can't modify the wallet concurrently | ||
| let exclusive_access = self.lock()?; | ||
| self.reload()?; | ||
| trace!("Wallet locked and loaded!"); | ||
|
|
||
| for cash_note in received_cash_notes { | ||
| let id = cash_note.unique_pubkey(); | ||
|
|
||
| if self.wallet.spent_cash_notes.contains(&id) { | ||
| debug!("skipping: cash_note is spent"); | ||
| continue; | ||
| } | ||
|
|
||
| if cash_note.derived_key(&self.key).is_err() { | ||
| debug!("skipping: cash_note is not our key"); | ||
| continue; | ||
| } | ||
|
|
||
| let value = cash_note.value()?; | ||
| self.wallet.available_cash_notes.insert(id, value); | ||
|
|
||
| self.store_cash_notes_to_disk(vec![cash_note])?; | ||
| } | ||
|
|
||
| self.store(exclusive_access)?; | ||
|
|
||
| Ok(()) | ||
| self.watchonly_wallet | ||
| .deposit_and_store_to_disk(received_cash_notes) | ||
| } |
There was a problem hiding this comment.
Same here, let's get rid of all that nesting
There was a problem hiding this comment.
see comment above, you are missing something
bochaco
force-pushed
the
feat-watch-only-wallet
branch
from
9bd8b0f
to
2d0e5e8
Compare
bochaco
force-pushed
the
feat-watch-only-wallet
branch
from
2d0e5e8
to
0985e88
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Summary generated by Reviewpad on 04 Dec 23 13:39 UTC
This pull request includes the following changes:
The file
sn_cli/src/subcommands/files/mod.rshas changes related to thefiles_cmdsfunction, theFilesconstructor, and theupload_filesfunction. Theclientparameter of these functions has been changed to a shared reference&Client.A new file
watch_only.rshas been added, which includes the implementation of aWatchOnlyWalletstruct. This struct represents a watch-only wallet in a SAFE Network and has various methods for loading, storing, and manipulating wallet data.The file
lib.rshas changes related to adding theWatchOnlyWalletstruct to thewalletmodule. This struct is now available for use in the API.The file
error.rshas a new variantPubKeyMismatchadded to theErrorenum, with a custom error message that includes the path of the wallet.The file
cashnote.rshas changes related to theCashNotestruct. A new methodderived_pubkeyhas been added, and the existing methodderivation_indexnow has a specified return type.The file
main.rshas changes related to the variablesjoins_gossipandjoins_gossipsub, as well as the functionfiles_cmds. These changes indicate updates to the usage of gossipsub and more efficient usage of theclientobject.The file
wallet_file.rshas changes related to thestore_created_cash_notesfunction, which now accepts a generic iterator and returns aResult<()>instead ofResult<()>. This allows for more flexibility in accepting different types of iterators.The file
error.rshas changes related to the addition of theMainPubkeyMismatchvariant to theErrorenum. This variant handles the scenario where the main public key does not match.The file
wallet/mod.rshas changes related to the usage of theMainPubkeytype and the addition of functions for storing and retrieving the main public key in the wallet directory.The file
wallet/mod.rshas changes related to the addition of a new module calledwatch_onlyand the implementation of aWatchOnlyWalletstruct. It also includes a new method calledbalancein theKeyLessWalletstruct.Please review these changes to ensure they are correct and do not introduce any bugs or performance issues. Let me know if you need further assistance.