Skip to content

feat:added fetching address lookup accounts implementation#85

Merged
MicaiahReid merged 3 commits intosolana-foundation:mainfrom
adpthegreat:add_alt_fetching
May 7, 2025
Merged

feat:added fetching address lookup accounts implementation#85
MicaiahReid merged 3 commits intosolana-foundation:mainfrom
adpthegreat:add_alt_fetching

Conversation

@adpthegreat
Copy link
Copy Markdown
Contributor

@adpthegreat adpthegreat commented May 4, 2025

Fixing tests (WIP), closes #84

@adpthegreat adpthegreat marked this pull request as draft May 4, 2025 08:55
Copy link
Copy Markdown
Collaborator

@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.

This is awesome work, thank you @adpthegreat!!

Could you also take a look at this line and see if we can include the address table lookups: https://github.com/txtx/surfpool/blob/0888995b9a73e0d5d12320c7ed183279ebe936e2/crates/core/src/types.rs#L91

I think it should be similar to what you've already done, but perhaps keeping the alt accounts separated from the message accounts in this part: https://github.com/txtx/surfpool/blob/0888995b9a73e0d5d12320c7ed183279ebe936e2/crates/core/src/types.rs#L58-L79

Comment thread crates/core/src/surfnet/mod.rs Outdated
Comment thread crates/core/src/tests/integration.rs Outdated

#[tokio::test]
async fn test_add_alt_fetching() {
let mock_tx = VersionedTransaction {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is awesome! Could you add in a comment or something indicating what this transaction or doing, or how the instructions/account_keys/address_lookup_table was derived, cause this is quite the intimidating struct 😰

@adpthegreat
Copy link
Copy Markdown
Contributor Author

@MicaiahReid

  • so i've added the ALTs to the types
  • removed the if statement from the account fetching part, then ran cargo fmt,
  • but for the tests, it can't work rn because the assertion i'm writing - (to check if all the pubkeys from the tx when fetched locally do not return None) uses surfnet_svm and its moved, so do i impl clone? want to get your feedback on everything i've done so far

@adpthegreat adpthegreat marked this pull request as ready for review May 6, 2025 18:13
@lgalabru
Copy link
Copy Markdown
Collaborator

lgalabru commented May 7, 2025

@MicaiahReid can we merge this PR?

@MicaiahReid
Copy link
Copy Markdown
Collaborator

@lgalabru, not yet, I'd like to see the test working, but I'm also having trouble thinking of how to accomplish this. I can investigate tomorrow

@MicaiahReid MicaiahReid merged commit fbc35d4 into solana-foundation:main May 7, 2025
2 checks passed
@MicaiahReid
Copy link
Copy Markdown
Collaborator

Great work @adpthegreat! I'll think on the tests and see what we can do, but we don't want to hold back the functionality any longer.

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.

Implement address lookup internally

3 participants