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

solana: fix lamport beneficiaries #109

Merged
merged 5 commits into from May 2, 2024
Merged

solana: fix lamport beneficiaries #109

merged 5 commits into from May 2, 2024

Conversation

a5-pickle
Copy link
Contributor

  • Execute order should send auction custody token account lamports to the owner of the initial offer token if it exists
  • Settle auction none should send prepared custody token lamports to prepared_by encoded in PreparedOrderResponse
  • Place order CCTP should send prepared order token account lamports to prepared_by encoded in PreparedOrder

Closes #104.

@a5-pickle a5-pickle force-pushed the solana/fix-prepared-by branch 4 times, most recently from 426e2e5 to 769790c Compare May 1, 2024 13:16
Copy link

@johnsaigle johnsaigle left a comment

Choose a reason for hiding this comment

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

I think the account close changes look good.

I'm curious the values are being extracted from ExecuteOrder now instead of passing the struct along directly as before?

@a5-pickle
Copy link
Contributor Author

a5-pickle commented May 2, 2024

I think the account close changes look good.

I'm curious the values are being extracted from ExecuteOrder now instead of passing the struct along directly as before?

Do you mean here?

auction: &'ctx mut Account<'info, Auction>,
fast_vaa: &'ctx LiquidityLayerVaa<'info>,
custody_token: &'ctx Account<'info, token::TokenAccount>,
config: &'ctx Account<'info, AuctionConfig>,
executor_token: &'ctx UncheckedAccount<'info>,
best_offer_token: &'ctx UncheckedAccount<'info>,
initial_offer_token: &'ctx UncheckedAccount<'info>,
.

I did this because I lazily passed a mutable reference to the whole ExecuteOrder composite before. I think passing each account from this context reflecting whether each account should be a static ref or mut ref feels better. What do you think?

As I look at this more, it's an interesting trade off of whether to pass specific references of each account from the composite. Or to pass the composite to leave it less error-prone to reference each account in the composite.

I guess inside the shared method I could specify static references to specific accounts. Is that preferred?

@a5-pickle
Copy link
Contributor Author

As I look at this more, it's an interesting trade off of whether to pass specific references of each account from the composite. Or to pass the composite to leave it less error-prone to reference each account in the composite.

I guess inside the shared method I could specify static references to specific accounts. Is that preferred?

Made the change here: 731df93. This does feel better. What do you think?

config,
best_offer_token,
} = active_auction;
let auction = &mut execute_order.active_auction.auction;
Copy link
Contributor

Choose a reason for hiding this comment

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

Look at all those references.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🐔

@a5-pickle
Copy link
Contributor Author

Going to merge this. @johnsaigle if you have a strong opinion about how the refs should be passed, I'll put up another PR after we discuss it.

@a5-pickle a5-pickle merged commit 799cd04 into main May 2, 2024
11 checks passed
@a5-pickle a5-pickle deleted the solana/fix-prepared-by branch May 2, 2024 23:35
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.

solana: Add a way to refund the payer of the custody_token when an offer is placed
3 participants