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.

Sign up for GitHub

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

Jump to bottom

Chunk /gettransactions to avoid hitting restricted RPC limit #8388

Merged
merged 1 commit into from
Jul 6, 2022
Merged

Chunk /gettransactions to avoid hitting restricted RPC limit #8388

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/wallet/wallet2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3159,14 +3159,18 @@ void wallet2::update_pool_state(std::vector<std::tuple<cryptonote::transaction,
}
}

// get those txes
if (!txids.empty())
// get_transaction_pool_hashes.bin may return more transactions than we're allowed to request in restricted mode
const size_t SLICE_SIZE = 100; // RESTRICTED_TRANSACTIONS_COUNT as defined in rpc/core_rpc_server.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we detect if the daemon is in restricted mode and chunk accordingly, as opposed to defaulting to the (rather low) value of 100?

Copy link
Contributor

@plowsof plowsof Jun 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

i think this is what happens already here (on the server side at least)

monero/src/rpc/core_rpc_server.cpp

Line 888 in 9750e1f

if (restricted && req.txs_hashes.size() > RESTRICTED_TRANSACTIONS_COUNT)

Copy link
Contributor

@jeffro256 jeffro256 Jun 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Right, but I mean but if the daemon is in unrestricted mode, then we can ask for more at once (what the old code was doing), which would definitely reduce overhead if/when the mempool gets to be the size of (for example) Bitcoin's with >1000 txs per block

for (size_t offset = 0; offset < txids.size(); offset += SLICE_SIZE)
{
cryptonote::COMMAND_RPC_GET_TRANSACTIONS::request req;
cryptonote::COMMAND_RPC_GET_TRANSACTIONS::response res;
for (const auto &p: txids)
req.txs_hashes.push_back(epee::string_tools::pod_to_hex(p.first));
MDEBUG("asking for " << txids.size() << " transactions");

const size_t n_txids = std::min<size_t>(SLICE_SIZE, txids.size() - offset);
for (size_t n = offset; n < (offset + n_txids); ++n) {
req.txs_hashes.push_back(epee::string_tools::pod_to_hex(txids.at(n).first));
}
MDEBUG("asking for " << req.txs_hashes.size() << " transactions");
req.decode_as_json = false;
req.prune = true;

Expand All @@ -3183,7 +3187,7 @@ void wallet2::update_pool_state(std::vector<std::tuple<cryptonote::transaction,
MDEBUG("Got " << r << " and " << res.status);
if (r && res.status == CORE_RPC_STATUS_OK)
{
if (res.txs.size() == txids.size())
if (res.txs.size() == req.txs_hashes.size())
{
for (const auto &tx_entry: res.txs)
{
Expand Down Expand Up @@ -3219,7 +3223,7 @@ void wallet2::update_pool_state(std::vector<std::tuple<cryptonote::transaction,
}
else
{
LOG_PRINT_L0("Expected " << txids.size() << " tx(es), got " << res.txs.size());
LOG_PRINT_L0("Expected " << n_txids << " out of " << txids.size() << " tx(es), got " << res.txs.size());
}
}
else
Expand Down