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

Adding abstraction of iteration over receivable entries #4496

Merged
merged 15 commits into from
Mar 18, 2024

Conversation

clemahieu
Copy link
Contributor

@clemahieu clemahieu commented Mar 16, 2024

This series of changes is aimed at decoupling queriers of receivable entries from the implementation of how they're stored. Queries of pending entries fall into three major categories

Ledger processing
This is an operation that checks the correctness of source blocks for blocks that receive a balance. If each block that sends a balance inserts an entry into an O(1) unordered_set (destination, block_hash), every receive block can be checked in O(1) time by using the (account, source) fields of blocks that receive. The existence of that value indicates the block is receivable.

Iterating blocks that can be received by an account.
This is done by a lot of wallet operations and RPCs related to wallet operations that want to check particular items available to be received for a particular account. A wallet would do this operation when looking for items it could process if wasn't signaled to that event through another means, e.g. websockets.

These operations are currently implemented with sorted queries over a single account searching for values. It might be possible to implement this without a sorted query.

Iterating accounts that have items to receive
The "unopened" rpc operates this way, iterating up to a certain number of accounts and summing the amount that can be received.

Iterating all items
Epoch upgrades, diagnostic functions

There are 3 functions that iterate over receivable entries:

ledger.receivable_upper_bound (tx, account) - This will search for the first receivable item for an account strictly greater than "account".
ledger.receivable_upper_bound (tx, account, hash) - This will search for the next receivable item for a particular account greater than "hash". This will return nullopt if iteration goes past items in this account.

Upper_bound is used for the convenience of being able to feed in the current value of an iteration without modification and still iterate entries.

ledger.receivable_lower_bound (tx, account, hash) - This search finds the first item equal to or greater than the items passed in. This is the base function used by upper_bound functions.

@clemahieu clemahieu marked this pull request as ready for review March 17, 2024 08:53
@clemahieu clemahieu force-pushed the receivable_iteration branch 4 times, most recently from 4a81e72 to 9bb5164 Compare March 17, 2024 09:37
auto i (node_a.store.pending.begin (transaction, nano::pending_key (random_account, 0)));
if (i != node_a.store.pending.end ())
auto item = node_a.ledger.receivable_upper_bound (transaction, random_account);
if (item)
Copy link
Contributor

Choose a reason for hiding this comment

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

Returns the next receivable entry for an account greater than 'account'

Is this condition checking the same thing as before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be. Since random_accout is a random number i.e. it's not an actual account that has a receivable entry, this is finding the next-biggest account that has a receivable entry.

size_t const pending_deque_overflow (64 * 1024);
for (auto i (node->store.pending.begin (transaction)), n (node->store.pending.end ()); i != n; ++i)
for (auto i = ledger.receivable_upper_bound (transaction, 0, 0); i; i = ledger.receivable_upper_bound (transaction, i.value ().first.account, i.value ().first.hash))
Copy link
Contributor

@pwojcikdev pwojcikdev Mar 18, 2024

Choose a reason for hiding this comment

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

I see ledger.receivable_upper_bound (transaction, i.value ().first.account, i.value ().first.hash)) pattern is used in quite a few places, it would be more readable and ergonomic to provide an override for upper_bound (..., <account, hash> pair)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. With the rewritten iterator classes there are now done with a normal ++i.

peers_l.put (key.hash.to_string (), info.amount.number ().convert_to<std::string> ());
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

An if/else decision tree is often easier to read than sequential code with continue statements.

pwojcikdev
pwojcikdev previously approved these changes Mar 18, 2024
@clemahieu
Copy link
Contributor Author

Rewrote these functions to return an iterator object to match standard c++ iteration.

@clemahieu clemahieu merged commit 186f3f4 into nanocurrency:develop Mar 18, 2024
17 of 27 checks passed
simpago added a commit to rsnano-node/rsnano-node that referenced this pull request Mar 24, 2024
The count limit got lost during the refactor in pull request nanocurrency#4496
simpago added a commit to rsnano-node/rsnano-node that referenced this pull request Mar 24, 2024
The count limit got lost during the refactor in pull request nanocurrency#4496
clemahieu pushed a commit that referenced this pull request Mar 24, 2024
…4520)

The count limit got lost during the refactor in pull request #4496
@qwahzi qwahzi added this to the V27 milestone Mar 25, 2024
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.

3 participants