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

Use indexation cache to satisfy "coins to spend" queries #2463

Open
wants to merge 249 commits into
base: master
Choose a base branch
from

Conversation

rafal-ch
Copy link
Contributor

@rafal-ch rafal-ch commented Nov 28, 2024

Closes #2391

This PR includes all changes from the Part 1 PR, making it deprecated.

Description

Changes in this PR:

The new CoinsToSpend index

  • This is the database that stores all coins to spend sorted by the amounts (i.e. largest-by-value coins first)
  • The key consists of several parts
    • Retryable flag - to distinguish between retryable messages and other coins
    • Address (owner)
    • AssetID
    • Amount - as "big-endian" bytes to leverage the RocksDB key sorting capabilities
    • Foreign Key - this are bytes of the key from either the Messages or Coins on-chain databases
      • for messages this is a 32-bytes Nonce
      • for coins this is a 34-bytes UtxoId
  • The value is an instance of IndexedCoinType enum, so we know which on-chain database to query when returning the actual coins
  • This index is updated when executor events are processed
  • When querying for "coins to spend" the following algorithm is applied:
    • First, we get as many "big" coins as required to satisfy double the amount from the query (respecting max and excluded params)
    • If we have enough coins, but there are still some "slots" in the query left (because we selected less coins than max) we fill the remaining slots with a random number of "dust" coins
    • If it happens that the value of selected "dust coins" is able to cover the value of some of the already selected "big coins", we remove the latter from the response
    • If at any step we encounter a problem (reading from database, integer conversions, etc.) we bail with an appropriate error

Changes to CoinsQueryError type

  • The MaxCoinsReached variant has been removed because in the new algorithm we never query for more coins than the specified max, hence, without additional effort, we are not able to tell whether the query could be satisfied if user provided bigger max
  • The InsufficientCoins has been renamed to InsufficientCoinsForTheMax and it now contains the additional max field

Off-chain database metadata

  • The metadata for off-chain database now contain the additional IndexationKind - CoinsToSpend

Refactoring

  • The indexation.rs module was split into separate files, each per indexation type + errors + some utils.

Other

  • Integration tests have to be updated to not expect the exact number of coins to spend in the response (currently, due to randomness, we're not deterministic in this regard)
  • The number of excluded ids in the coinsToSpend GraphQL query is now limited to the maximum number of inputs allowed in transaction.

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

Follow-up issues

@rafal-ch rafal-ch requested a review from xgreenx December 13, 2024 12:54
@netrome netrome self-requested a review December 16, 2024 15:14
Copy link
Contributor

@netrome netrome left a comment

Choose a reason for hiding this comment

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

Nice stuff so far. I have only done a relatively shallow sweep and will get back again during the week for a deeper look. I have a few nits and suggestions so far, but nothing major.

I'm not a fan of the lack of explicit error handling in crates/fuel-core/src/graphql_api/storage/coins.rs but other than that I think the code looks generally really well-written.

crates/fuel-core/src/coins_query.rs Show resolved Hide resolved
crates/fuel-core/src/graphql_api.rs Show resolved Hide resolved
crates/fuel-core/src/graphql_api/indexation/balances.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/graphql_api/indexation/balances.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/graphql_api/indexation/balances.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/graphql_api/storage/coins.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/graphql_api/storage/coins.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/graphql_api/storage/coins.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/graphql_api/storage/coins.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/graphql_api/storage/coins.rs Outdated Show resolved Hide resolved
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.

Optimize GraphQL "coins to spend" query
5 participants