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

monero-wallet-cli: Added comman scan_tx #7312

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

hMihaiDavid
Copy link

Please see gh-7291

src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
@hMihaiDavid
Copy link
Author

Had to convert txids from std::set to std::vector to be able to use indexing. Tell me if there's a more efficient way and I'll refactor it.

src/simplewallet/simplewallet.cpp Show resolved Hide resolved
src/simplewallet/simplewallet.cpp Outdated Show resolved Hide resolved
src/simplewallet/simplewallet.cpp Outdated Show resolved Hide resolved
src/simplewallet/simplewallet.cpp Show resolved Hide resolved
src/simplewallet/simplewallet.cpp Outdated Show resolved Hide resolved
src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
@Gingeropolous
Copy link
Collaborator

this is gonna revolutionize mobile wallet experience.

so, does this allow a wallet to ask a daemon for a specific set of blocks?

or is it asking for a specific tx_id?

I was really excited about your previous comment from the proposal thread:

After resolving this command, I'd like to add a similar command for block heights instead of txids. My initial idea was to write on paper some information for fast recovery of a paper wallet, and tx hashes seem very long.

src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
@hMihaiDavid
Copy link
Author

Replying to @Gingeropolous

this is gonna revolutionize mobile wallet experience.

Please elaborate more, I'm interested (I thought most mobile wallets were light wallets).

so, does this allow a wallet to ask a daemon for a specific set of blocks?
or is it asking for a specific tx_id?
I was really excited about your previous comment from the proposal thread:

After resolving this command, I'd like to add a similar command for block heights instead of txids. My initial idea was to write on paper some information for fast recovery of a paper wallet, and tx hashes seem very long.

The feature so far (scan_tx), allows for asking only for a specific set of txids, not blocks.
When this is done I'd love to continuing adding the same feature but for block heights. It should not be that difficult after doing the scan_tx.

Would you prefer getting blocks by block hashes instead of heights? Is this more useful for the mobile wallets?. I think I could add both apis, for block heights and block hashes, since the rpc allows to get blocks by both identifiers.

@Gingeropolous
Copy link
Collaborator

Please elaborate more, I'm interested (I thought most mobile wallets were light wallets).

Mobile wallets are light wallets. The wallet connects to a remote daemon over daemon RPC. So currently, when you create a wallet on your phone using this architecture, the phone has to request all of the blocks from 0 that it wants to scan. Even if you create a new wallet, it retrieves something and still takes a while even though there's no need to scan those blocks. Try it for yourself using cakewallet on iphone or android.

Additionally, after your wallet is synchronized to the daemon, you generally just close the wallet and don't use it for a week or months. Now, when you start up your wallet again, the wallet has to request the intervening 2 weeks of blocks from the daemon in order for the wallet to create transactions again. This is ridiculous, because you, the user, knows that you didn't use your wallet in those 2 weeks, so there's no reason to scan those blocks.

Thus, when opening a dormant wallet and the user just wants to create a transaction immediately, the user could just hit a "fast forward" button, so that the wallet just requests the last block or the last 5 blocks (or no blocks), and then the user can start crafting transactions immediately instead of having to wait for the wallet to refresh. This provides better UX and also reduces the load on the remote nodes.

or, if the user did know the received a transaction on january 20th, and they just opened their wallet today, they could tell the wallet to just request blocks from that date. This would cut down significantly on the server load for remote nodes, and provide better user experience.

Would you prefer getting blocks by block hashes instead of heights? Is this more useful for the mobile wallets?. I think I could add both apis, for block heights and block hashes, since the rpc allows to get blocks by both identifiers.

I think heights is the easiest bet for the use case i'm imagining, but block hash would probably be useful as well.

@hMihaiDavid
Copy link
Author

@Gingeropolous

Nice. Thanks for the explanation. I used to think light wallet ran all the blockchain scanning on the serverside with the client just signing txs, I didn't know they connect over rpc to a remote monerod. It's more privacy-preserving this way.

I checked and it seems that cakewallet doesn't use this official wallet-cli code for the wallet, so this feature (scan_tx, scan_block) would have to be implemented there too. Seems like a good idea, it would serve as the basis for the fast-forward mechanism.

@hMihaiDavid
Copy link
Author

Also btw, I don't do the "check_rpc_cost" stuff after the node rpc call. Should I look into that? Thx.

@Gingeropolous
Copy link
Collaborator

Also btw, I don't do the "check_rpc_cost" stuff after the node rpc call. Should I look into that? Thx.

Yes.... maybe? Are you familiar with rpc-pay? I assume check_rpc_cost is associated with the rpc-pay function. Briefly, it allows a remote node user (someone connecting with a wallet) to provide mining hashes towards a block the remote node operator (running the daemon) is mining on. So, its a way for a user to pay for services. No one uses it right now because its not really implemented into any wallets besides the CLI. I think once its in the GUI though it'll be more of a thing.

@moneromooo-monero
Copy link
Collaborator

Please squash.

To implement this feature, the wallet2::scan_tx API was implemented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants