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

runtime: Move From<AddressLookupError> from sdk #141

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

joncinque
Copy link

@joncinque joncinque commented Mar 8, 2024

Problem

As part of the migration of the address-lookup-table program to BPF, we've had to also port over the conversion from AddressLookupError to AddressLoaderError, but it's only needed in one place in the runtime.

See solana-program/address-lookup-table#7 (comment) some more info

Summary of Changes

Move the error conversion function to where it's used in the runtime. The main motivation for moving it up rather than keeping it in the SDK is to avoid a circular dependency between solana-program and the new address-lookup-table BPF crate.

Let me know if this works with your conception of address loaders. It does seem like structs implementing AddressLoader might want the error converter since they'll be using lookup tables, but I'm not sure of a better place to put it.

Alternatively, we could create a solana-message crate which has all of the sdk/program/src/message code, and re-export it in solana-sdk. That crate could safely depend on solana-program and the new address-lookup-table crate.

cc @buffalojoec

Fixes #

@joncinque joncinque requested a review from jstarry March 8, 2024 00:47
@joncinque joncinque changed the title sdk: Move From<AddressLookupError> into runtime runtime: Move From<AddressLookupError> from sdk Mar 8, 2024
Copy link

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

Yeah this seems like a good place for it

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 36.36364% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 81.8%. Comparing base (8887cd1) to head (dd9f38b).
Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #141     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         838      838             
  Lines      225950   225949      -1     
=========================================
- Hits       184979   184945     -34     
- Misses      40971    41004     +33     

@joncinque joncinque merged commit 377e1f9 into anza-xyz:master Mar 8, 2024
45 checks passed
@joncinque joncinque deleted the alterror branch March 8, 2024 10:02
codebender828 pushed a commit to codebender828/agave that referenced this pull request Oct 3, 2024
sdk: Move `From<AddressLookupError>` into runtime
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