Skip to content

Feat: allow arbitrary programs in swap transactions #582

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

anihamde
Copy link
Contributor

@anihamde anihamde commented Aug 5, 2025

This PR allows arbitrary program instructions in swap transactions.

Previously, the server maintained effectively a whitelist of program instructions that were allowed to be included in swap transactions, in order to protect users from signing transactions with unintended consequences.

In order to allow arbitrary program instructions while maintaining security, this PR changes the code to allow any of the aforesaid approved program instructions and allow an arbitrary program instruction provided that it does not invoke the user account (which is a signer of the transaction) in any of its accounts (including lookup table accounts). If this second condition is met, the user's signature on the transaction should not be actionable towards any logic in the arbitrary program instruction.

Some outstanding TODOs:

  • make the extraction of lookup table accounts more efficient by bundling multiple lookups into a single call
  • fix transfer checks to allow any transfers that do not invoke the user account

Copy link

vercel bot commented Aug 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
swap-staging Ignored Ignored Preview Aug 15, 2025 5:01pm

.await;
let program_id = instruction.program_id;
let transaction = Transaction::new_with_payer(
&[instruction.clone(), swap_instruction.clone()],
Copy link
Contributor

@guibescos guibescos Aug 12, 2025

Choose a reason for hiding this comment

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

i don't think you need swap_instruction here and in the other tests that only call check_approved_program_instruction

) -> Result<(), RestError> {
if self
.check_approved_program_instruction(program_id, ix)
.is_ok()
Copy link
Contributor

Choose a reason for hiding this comment

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

it's doesn't feel great that now we swallow the error from check_approved_program_instruction. this might make it harder to debug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i agree. i refactored this to bubble up the error from check_approved_program_instruction if the user account is invoked

Copy link
Contributor

@guibescos guibescos left a comment

Choose a reason for hiding this comment

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

lgtm, the relayer stuff feels a bit paranoid

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.

2 participants