wormhole-foundation / example-liquidity-layer

Apache License 2.0
11 stars 11 forks source link

solana: fix lamport beneficiaries #109

Closed a5-pickle closed 4 months ago

a5-pickle commented 4 months ago

Closes #104.

a5-pickle commented 4 months ago

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? https://github.com/wormhole-foundation/example-liquidity-layer/blob/769790ca7c96eb997c6376fc1c0ac10e773afb27/solana/programs/matching-engine/src/processor/auction/execute_fast_order/mod.rs#L21-L27.

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 commented 4 months ago

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: https://github.com/wormhole-foundation/example-liquidity-layer/pull/109/commits/731df932494c54f879d53d819de429072775afa2. This does feel better. What do you think?

a5-pickle commented 4 months ago

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.