Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feature: CPI Events API #2438
Feature: CPI Events API #2438
Changes from 7 commits
1b9ff06
eed5e6f
6c9fced
e1d234c
f5c5ab9
bca57d6
ffd6fb1
aadba4e
bbd869b
7bc2c25
3d04a71
014256f
76ee7be
f4c225c
25bc040
4786e85
b072f59
0b678cf
b7ccc2a
8c2fbdc
e68be79
d9cd325
f9bfeff
5634cef
9cd071f
8b7f515
1f456ea
fe49ef6
397108b
cdd9776
8cc2642
690e8c7
66d72b1
87652d2
eb051ef
fd182fe
852b8bd
aa11826
ac45fc7
9b17a4c
22b902a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example https://github.com/Ellipsis-Labs/phoenix-v1/blob/ee9cc0333fbab6ee20861e51201951b3e9c38dc9/src/lib.rs#L104-L124
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about two different macros,
emit_cpi!
andemit_cpi_signed!
?I'll look into the introspection stuff.
I think devs should have options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why
log_authority
is even needed here?Are people writing indexers that parse
ix
s from TransactionStatus Metadata separately from their transaction context? Is that the norm? I don't understand howlog_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?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.