Feature: CPI Events API

[ngundotra]

Adds emit_cpi to allow programs to store logs in CPI data, the same way Metaplex's compressed NFTs do.

Closes #2408

[vercel]

@ngundotra is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

[ngundotra]

Unfortunately I cannot move the _emit_cpi_data and emit_cpi macro into the lang/attribute crate because attribute crate is a proc_macro crate, and emit_cpi is a declarative macro, which are incompatible exports. ie attribute can only export proc_macro macros 🙃

[ngundotra]

Rewrote emit_cpi as a proc_macro that takes a tuple (AccountInfo, <dyn anchor_lang::Event>)

[armaniferrante]

Should we be using the instructions sysvar to check that this call was self-referential, i.e., that the previous instruction was to this program? Do we want to allow other programs or even top level instructions to call this?

[Arrowana]

An example https://github.com/Ellipsis-Labs/phoenix-v1/blob/ee9cc0333fbab6ee20861e51201951b3e9c38dc9/src/lib.rs#L104-L124

[ngundotra]

how about two different macros, emit_cpi! and emit_cpi_signed! ?

I'll look into the introspection stuff.

I think devs should have options

[Arrowana]

i would say log authority is better, it is also an account to be provided but much simpler to check.
What would be the advantage of the ix introspection?
I don't feel dev should have the option when there is no advantage to a solution, it only bloats anchor.

[ngundotra]

I don't understand why log_authority is even needed here?

Are people writing indexers that parse ixs from TransactionStatus Metadata separately from their transaction context? Is that the norm? I don't understand how log_authority helps indexers? Sure it prevents other programs from invoking that instruction, but it will still show up in getSignaturesForAddress.

Can you provide a concrete example of a program that benefits from having log_authority guarding the event ix?

[Arrowana]

You need to ask @jarry-xiao, I think it is mainly to avoid an extra burden for log parsers yes, to have to check that the caller is indeed the correct program. So it might not be 100% necessary

[jarry-xiao]

It’s not 100% necessary, but it prevents footguns (which I think is one of the key tenets of anchor).

We’ve chatted about this before, and I gave a concrete example of making a CPI from a rogue program into the log instruction of the target program. The parser can defend against this but the logic becomes more error prone.

I think it’s fine if you provide an indexing example of how one might process this along with a test case of how it blocks out erroneous instances of calling the log instruction (both through top level transaction calls and CPIs from rogue programs)

[jarry-xiao]

Also log_authority means you don’t need introspection, which I think is a win. To my knowledge, you still need to explicitly pass in SysvarInstructions

[jarry-xiao]

Here’s a super concrete example. Suppose you have a target program T and a rogue program R. R first makes a normal call to T, which will emit a regular event. Then in the same instruction it makes a call to T’s log instruction. You can figure out this is erroneous if you have the tx stack depth, but I think it’s ambiguous in the current interface.

In a vacuum you also know nothing about T’s logging patterns. Maybe it calls emit_cpi! a variable number of times. The point is there’s no way to know (again, until stack depth is in the transaction object which could realistically take months)

However if R’s call to T’s log instruction fails 100% of the time, then there’s nothing to ever worry about

[Arrowana]

Another point for the log authority, if your program allows arbitrary cpi (or only arbitrary self cpi) for any reason (flash loan capability...), then someone can write random logs without the log authority.

[austbot]

I would couple this to the 1.15 release where we get stack height from geyser. Or have a huge disclaimer that without log parsing you ita tricky to know at what depth the cpi was made

[acheroncrypto]

Thank you for addressing previous comments. I've left a few more comments, some of them are pretty important. Let me know what you think!

[acheroncrypto]

Here is the summary for usage with my last changes:

Instruction

pub fn my_instruction(ctx: Context<MyInstruction>) -> Result<()> {
-    emit_cpi!(
-        ctx.accounts.program.to_account_info(),
-        ctx.accounts.event_authority.clone(),
-        *ctx.bumps.get("event_authority").unwrap(),
-        MyEvent { data: 5 }
-    );
+    emit_cpi!(MyEvent { data: 5 });
    Ok(())
}

Accounts

+#[event_cpi]
#[derive(Accounts)]
-pub struct MyInstruction<'info> {
-    /// CHECK: this account is needed to guarantee that your program is the one doing the logging
-    #[account(seeds=[b"__event_authority"], bump)]
-    pub event_authority: AccountInfo<'info>,
-    pub program: Program<'info, crate::program::MyProgramName>,
+pub struct MyInstruction {}

Client

-await program.methods
-.myInstruction()
-.accounts({
-  eventAuthority: anchor.web3.PublicKey.findProgramAddressSync(
-    [Buffer.from("__event_authority")],
-    program.programId
-  )[0],
-  program: program.programId,
-})
-.rpc();
+await program.methods.myInstruction().rpc();

[acheroncrypto]

Last changes:

• Anyone was able to invoke the event handler before 87652d2. We're now validating the event authority signed the transaction and is correct key in the event handler instruction, added a test case for this so that only the program itself can invoke the event handler.
• Removed the #[account(address = crate::ID)] check from the main instruction because we're invoking with crate::ID as the program id and it will fail if the program wasn't passed in.
• Renamed the no-cpi-eventsfeature to event-cpi and made the feature opt-in instead of opt-out.

@ngundotra let me know what you think.

[ngundotra]

This works great! The emit_cpi!{} macro just works, and the #[event_cpi] attr is great.

I think we need 2 changes to the JS sdk tho.

1. We need a method like program.cpiEvents.parseTransaction() and gives a list of events from a transaction
2. Probably the eventAuthority address derivation on program.addresses.eventAuthority (and probably export IDL similarly like program.addresses.idl) - probably can be done in another PR

What do you think?

[acheroncrypto]

I think we need 2 changes to the JS sdk tho.

1. We need a method like program.cpiEvents.parseTransaction() and gives a list of events from a transaction
2. Probably the eventAuthority address derivation on program.addresses.eventAuthority (and probably export IDL similarly like program.addresses.idl) - probably can be done in another PR

What do you think?

Yeah client side needs improvement and I think both of those can be made in a separate PR.

One potential issue here is that we are deriving the key each time the event handler is called to validate the authority(on top of deriving it on the main instruction) but the authority key and bump does not change and can be hard coded inside the program. The solution would most likely be a similar macro to declare_id! which kind of sucks. I'll come back to this if it causes noticable difference in compute units.

I've added and updated the documentation and looking to merge this soon if it looks good to you as well.

[ngundotra]

Awesome, that sounds like a plan.

Thanks for updating documentation, your changes look great to me. I tested externally by importing in another standalone repo, and it works like a charm.

🚢 it