-
Notifications
You must be signed in to change notification settings - Fork 47
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
refactor: Remove hard dependency on light-system-program
in SDK
#1230
Conversation
5ea2fe6
to
99647a0
Compare
First of all, add more types to the SDK, which are going to be used by third-party applications, which were until now defined in program crates: - `address` - `NewAddressParams` - `NewAddressParamsPacked` - `compressed_account` - `CompressedAccount` - `CompressedAccountData` - `CompressedAccountWithMerkleContext` - `PackedCompressedAccountWithMerkleContext` - `OutputCompressedAccountWithPackedContext` - `event` - `MerkleTreeSequenceNumber` - `PublicTransactionEvent` - `merkle_context` - `QueueIndex` - `MerkleContext` - `PackedMerkleContext` - `proof` - `CompressedProof` - `ProofRpcResult` - `verify` - `CompressedCpiContext` - `InstructionDataInvokeCpi` Hide the imports from `light_system_program` behind a feature flag. The long-term plan is to remove them all together. Provide a dependency-free, tinier implementation of `TestIndexer` in the `light-client` crate.
99647a0
to
b3a8299
Compare
@@ -377,11 +362,11 @@ mod tests { | |||
pub my_program: Program<'info, MyProgram>, // Missing #[self_program] | |||
pub payer: Signer<'info>, // Missing #[fee_payer] | |||
pub user: AccountInfo<'info>, // Missing #[authority] | |||
pub light_system_program: Program<'info, LightSystemProgram>, | |||
pub registered_program_pda: Account<'info, RegisteredProgram>, | |||
pub light_system_program: AccountInfo<'info>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to check this account since this is the program id we cpi. It could be a manual check in the verify method to avoid the program dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other compression specific accounts don't need to be checked because we check them inside of the system program invocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done da7be0d
let none_account_info = ctx.accounts.get_light_system_program().to_account_info(); | ||
|
||
let (cpi_context_account_info, cpi_context_account_meta) = | ||
match ctx.accounts.get_cpi_context_account() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are more accounts that are optional, do we require these to be always Some()
in the current implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah we currently just set those accounts to None
by default with none_account_info
(sol_pool_pda, decompression_recipient).
We need these other accounts too for full functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two accounts which we always set to None
for now:
sol_pool_pda
decompression_recipient
Otherwise, everything else is Some
I want to handle them in the future, but abstractions for token accounts are still something we need to design properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is independent from token accounts this is for sol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's still for compressed SOL transfers, right? And here, again, we need to design good abstractions.
First of all, add more types to the SDK, which are going to be used by third-party applications, which were until now defined in program crates:
address
NewAddressParams
NewAddressParamsPacked
compressed_account
CompressedAccount
CompressedAccountData
CompressedAccountWithMerkleContext
PackedCompressedAccountWithMerkleContext
OutputCompressedAccountWithPackedContext
event
MerkleTreeSequenceNumber
PublicTransactionEvent
merkle_context
QueueIndex
MerkleContext
PackedMerkleContext
proof
CompressedProof
ProofRpcResult
verify
CompressedCpiContext
InstructionDataInvokeCpi
Hide the imports from
light_system_program
behind a feature flag. The long-term plan is to remove them all together.Provide a dependency-free, tinier implementation of
TestIndexer
in thelight-client
crate.