wallet2/wallet-rpc: Immediate lookahead table expansion + set_subaddresss_lookahead RPC endpoint #8981
Conversation
There was a problem hiding this comment.
Afaik you can start iterating from the current lookahead position (subaddress labels size + lookahead). It should be fine to call create_one_off_subaddress() for all new subaddresses in the lookahead delta (there shouldn't be duplicates, or no?). That way you don't need the inverted subaddresses map.
src/wallet/wallet2.cpp
Outdated
| for (const auto& pair : m_subaddresses) { | ||
| subaddresses_inverted[pair.second] = pair.first; // inverting the keys and values makes it easier to lookup indices | ||
| } | ||
| for (uint32_t i = 0; i < m_subaddress_labels.size()+major-1; i++) { // m_subaddress_labels tells us how many accounts the user has actually requested without lookahead |
There was a problem hiding this comment.
This overflows if labels.size() and major equal 0. Better to do i + 1 < ... (same for the minor loop below).
There was a problem hiding this comment.
Just slightly outside of the diff, the first thing this function does is assert that major is greater than zero and .size returns an unsigned int :) I still changed it
|
You're right, thank you. I think the expressions I used this time only make as many calls to |
benevanoff
force-pushed
the
fix-set-subaddr-lookahead
branch
from
f147882 to
d6340d7
Compare
benevanoff
force-pushed
the
fix-set-subaddr-lookahead
branch
from
d6340d7 to
2440f18
Compare
benevanoff
force-pushed
the
fix-set-subaddr-lookahead
branch
from
2440f18 to
8123d94
Compare
benevanoff
force-pushed
the
fix-set-subaddr-lookahead
branch
4 times, most recently
from
d07b6f6 to
e97736e
Compare
benevanoff
force-pushed
the
fix-set-subaddr-lookahead
branch
from
17d86ba to
975f7b7
Compare
|
OK, I made a mess of the changelog but it should be ready for review now :) |
tests/unit_tests/subaddress.cpp
Outdated
| test_idx = {100, 299}; | ||
| subaddr = w1.get_subaddress(test_idx); | ||
| EXPECT_NE(boost::none, w1.get_subaddress_index(subaddr)); | ||
| } |
There was a problem hiding this comment.
Minor but please add a newline here.
benevanoff
force-pushed
the
fix-set-subaddr-lookahead
branch
from
975f7b7 to
0be321f
Compare
benevanoff
changed the title
|
I've updated the PR to address #8954 as well. I thought about opening a separate PR but figured I would just lump it in here since the RPC endpoint depends on these changes to wallet2. I hope that's alright, but I can still break them apart if y'all would prefer. |
|
Will be nice to have this implemented. Thanks. |
In that case, And just confirming, there's no other way to set the wallet's subaddress lookahead through monero-wallet-rpc to avoid that, right? Something is better than nothing in that case since it can be manually set when opened, but of course not ideal. |
|
@benevanoff @woodser @selsta @UkoeHB @EdwardBetts Hi everyone, I'm interested in this feature and wanted to ask if there's any update on whether this PR will be merged? It seems like all the feedback has been addressed, but it hasn't been accepted yet. Additionally, is there currently any other way to change the scanned subaddress range via RPC? This would be extremely helpful for managing large wallets. Thank you! |
|
Continuing in #9953 |
A fix for #8980 coupled with an implementation of feature request #8954
Basically the current behavior is that if you make a wallet with the default settings in the cli and then do
set subaddress-lookahead 50:300, then the subaddress pubkey table hasnt actually expanded and you aren't looking any further when you scan. Instead, the table will get expanded to this look ahead only after either a new enote is found with the old lookahead or if the user requests a new subaddress withaddress new.This patch makes sure that the wallet adjusts the pubkey table immediately after sending the set command if an increase in the lookahead is requested.
This patch also adds a new endpoint to the wallet-rpc program,
/set_subaddress_lookaheadwhich allows the user to expand the subaddress lookahead table via remote command. As mentioned in the original ticket, the change does not persist to disk when the wallet closes and changing the wallet-rpc program to save those cached values looks like kinda a pain but I hope that the requestor finds that something is better than nothing for now.