solana: emit -> emit_cpi

[a5-pickle]

Instead of using emit! by itself, we use emit_cpi! for all events. Only the AuctionUpdated event (emitted during the auction offer process) is written to the program logs in addition to event CPI.

This PR also adds an event listener in the Matching Engine SDK (onEventCpi) for the events that no longer log anything in program logs.

This is a breaking change. This PR modifies some event schemas to give more useful information. The Matching Engine SDK takes into account these schema changes. The following event schemas changed:

pub struct AuctionSettled {
    pub fast_vaa_hash: [u8; 32], // used to be `auction: Pubkey`
}

pub struct AuctionUpdated {
    pub config_id: u32,
    pub fast_vaa_hash: [u8; 32], // used to be `auction: Pubkey`
    ..
}

pub struct FastFillRedeemed {
    pub prepared_by: Pubkey,
    pub fast_fill: FastFillSeeds, // used to be `fast_fill: Pubkey`
}

pub struct OrderExecuted {
    pub fast_vaa_hash: [u8; 32], // used to be `auction: Pubkey`
    ..
}

[a5-pickle]

There's one instance that should still be changed IMO:

solana/programs/matching-engine/src/processor/fast_fill/complete.rs
82-
83-    // Emit event that the fast fill is redeemed. Listeners can close this account.
84:    emit!(crate::events::FastFillRedeemed {
85-        prepared_by: ctx.accounts.fast_fill.info.prepared_by,
86-        fast_fill: ctx.accounts.fast_fill.key(),

In addition there are several places in the codebase using msg! for logging. These case seem mostly to be outside of the general auction flow, and so might not have a security impact.

msg!() is mostly used in the context of executing admin actions. In this case a malicious admin could try to hide these artifacts. That said there is also a non-logging version of this function so they could just use this instead. In any case, there are other means for detecting an upgrade has changed.

So I think that this fix looks good so long as the last emit!() is addressed.

How does this look? https://github.com/wormhole-foundation/example-liquidity-layer/pull/187/commits/234d238229c0b0815f541cd4a23b152236d85896

[mdulin2]

Is there any worry about too many nested calls? I think Solana has a maximum call depth of 4, where the emit_cpi does count for one of these.

[mdulin2]

With using Swap Layer, I want to make sure that the following isn't possible:

Payload Swap layer caller ->Swap Layer -> Token Router -> Matching Engine-> Emit CPI via nested contract call (goes over limit)

[johnsaigle]

@mdulin2 In that case, I think we'd expect the integration tests to fail on the Swap Layer side unless something is mocked in such a way that the calls don't really happen. Any thoughts on this @a5-pickle ?

[a5-pickle]

@mdulin2 good call out regarding the call depth.

When @johnsaigle asked for the complete fast fill event to be converted to event CPI, I was thinking about the implication of this.

TL;DR nothing is affected by adding this event CPI.

We imagined that a program integrating with the Token Router would be composing with the consume prepared fill instruction (not the redeem fast fill instruction). Redeem fast fill creates a prepared fill, which needs to be consumed using the consume prepared fill instruction (and we did it this way to address the call depth issue). Check out swap layer's complete transfer/swap instructions as an example of how this works (and swap layer has its own redeeming logic spread out across complete transfer/swap and release inbound instructions).

But to address the call stack with redeem fast fill:

Redeem fast fill (Token Router) -> complete fast fill (Matching Engine) -> event CPI.

It was already at this call depth due to SPL Token program's transfer call. So having this event CPI should not add to the call depth.

[mdulin2]

Both logs and CPI events are being used. From internal chats, this is intentional because real time notifications are not possible through CPI events. So, the thought is that the regular logs can be the initial attempt but there would be a fallback mechanism similar to how the Wormhole Guardian works to see these anyway.

There's not really a good mechanism to fix this besides asking integrators to do multiple things to prevent weird auction related attacks.