8000 feat: added new account indexes by Ozodimgba · Pull Request #151 · txtx/surfpool · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: added new account indexes #151

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
Merged
merged 4 commits into from
Jun 17, 2025
Merged

feat: added new account indexes #151

merged 4 commits into from
Jun 17, 2025

Conversation

Ozodimgba
Copy link
Contributor
@Ozodimgba Ozodimgba commented Jun 8, 2025 •
edited
Loading

This Draft aim to replace the current single account_registry with multiple specialized indexes to enable fast lookups of token accounts by owner, delegate, and mint without duplicating account data in memory following @MicaiahReid.
Problem
The current implementation has limitations when querying token accounts:

Missing Token Queries: No efficient way to find token accounts by delegate or mint
Slow Lookups: Would require iterating through all accounts to find specific token accounts

// Current approach - duplicates account data
pub account_registry: HashMap<AccountOwner, Vec<(Pubkey, Account)>>,

Solution
Replace the single registry with multiple specialized indexes that store accounts once and provide fast lookups:

// New approach - single storage, multiple indexes
pub accounts_registry: HashMap<Pubkey, Account>,                    // Single source of truth
pub accounts_by_owner: HashMap<Pubkey, Vec<Pubkey>>,               // program_id -> account_pubkeys  
pub token_accounts: HashMap<Pubkey, TokenAccount>,                 // pubkey -> parsed_token_data
pub token_accounts_by_owner: HashMap<Pubkey, Vec<Pubkey>>,         // owner -> token_account_pubkeys
pub token_accounts_by_delegate: HashMap<Pubkey, Vec<Pubkey>>,      // delegate -> token_account_pubkeys
pub token_accounts_by_mint: HashMap<Pubkey, Vec<Pubkey>>,          // mint -> token_account_pubkeys

@Ozodimgba Ozodimgba marked this pull request as draft June 8, 2025 13:41
@LevBeta
Copy link
LevBeta commented Jun 11, 2025

The optimization idea is solid, but I'm not a big fan of introducing multiple new hash maps that store owned Pubkey values both as keys and within their entries—especially since many of these values will end up duplicated or even triplicated.

For example the

pub token_accounts: HashMap<Pubkey, TokenAccount>,                 // pubkey -> parsed_token_data
pub token_accounts_by_owner: HashMap<Pubkey, Vec<Pubkey>>,         // owner -> token_account_pubkeys
pub token_accounts_by_delegate: HashMap<Pubkey, Vec<Pubkey>>,      // delegate -> token_account_pubkeys
pub token_accounts_by_mint: HashMap<Pubkey, Vec<Pubkey>>,          // mint -> token_account_pubkeys

Assuming N token accounts, we're storing roughly N * 3 extra repeated owned Pubkey values just for indexing. This leads to unnecessary memory usage and allocation overhead.

@MicaiahReid
Copy link
Member

The optimization idea is solid, but I'm not a big fan of introducing multiple new hash maps that store owned Pubkey values both as keys and within their entries—especially since many of these values will end up duplicated or even triplicated.

For example the

pub token_accounts: HashMap<Pubkey, TokenAccount>,                 // pubkey -> parsed_token_data
pub token_accounts_by_owner: HashMap<Pubkey, Vec<Pubkey>>,         // owner -> token_account_pubkeys
pub token_accounts_by_delegate: HashMap<Pubkey, Vec<Pubkey>>,      // delegate -> token_account_pubkeys
pub token_accounts_by_mint: HashMap<Pubkey, Vec<Pubkey>>,          // mint -> token_account_pubkeys

Assuming N token accounts, we're storing roughly N * 3 extra repeated owned Pubkey values just for indexing. This leads to unnecessary memory usage and allocation overhead.

Agreed on the downsides, @LevBeta, but this will work for now.

Any suggestions for future optimizations on this approach?

@Ozodimgba Ozodimgba changed the title WIP: account indexs feat: added new account indexs Jun 16, 2025
@Ozodimgba Ozodimgba marked this pull request as ready for review June 16, 2025 23:34
@Ozodimgba
Copy link
Contributor Author

@MicaiahReid
1: added new indexs following your directions. The PR is a major dependency for the endpoints => getTokenAccountsByDelegate, getTokenAccountsByOwner as this impact their implementation.

2: if we going to be frequently checking for duplicates, we can use HashSet instead of Vec for the indexes. This makes duplicate checking O(1) instead of O(n). (not a blocker at all just a suggestion)

@MicaiahReid MicaiahReid changed the title feat: added new account indexs feat: added new account indexes Jun 17, 2025
MicaiahReid
MicaiahReid approved these changes Jun 17, 2025
View reviewed changes
Copy link
Member
@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

LGTM, @Ozodimgba!

One small thing is that we are only parsing token accounts, not token 2022. Would be an easy thing for me to add later, so I'd rather not block.

Also you've just got some merge conflicts to resolve since your other PR was merged!

@Ozodimgba
Copy link
Contributor Author

@MicaiahReid resolved here on github...it should build sucessfully. If it doesnt will just sort it out first thing tomorrow before I commit the other endpoints on WIP

@Ozodimgba
Copy link
Contributor Author

@lgalabru need you to review this so it can get merged...I need it for the token endpoints

lgalabru
lgalabru approved these changes Jun 17, 2025
View reviewed changes
Copy link
Member
@lgalabru lgalabru left a comment

Choose a reason for hiding this comment

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

Thanks @Ozodimgba 🙏

@lgalabru lgalabru merged commit 1d6c5d9 into txtx:main Jun 17, 2025
2 checks passed
BretasArthur1 pushed a commit that referenced this pull request Jun 25, 2025
@Ozodimgba @BretasArthur1
feat: added new account indexes (#151)
d339684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgalabru lgalabru lgalabru approved these changes

@MicaiahReid MicaiahReid MicaiahReid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ozodimgba @LevBeta @MicaiahReid @lgalabru
0