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

Fix: Event listener not firing when creating associated accounts #397

Merged

Conversation

Standaa
Copy link
Contributor

@Standaa Standaa commented Jun 16, 2021

The code makes the assumption that a program log will always chronologically "follow" it's governing program.
Nonetheless, when the same transaction also initializes an associated account, this order gets mixed up.
The last program to be used becomes a cpi.

Let's take an example:

Logs for a simple transaction might look like:

[
  'Program 3F3r9Njk8YkVJVyaxW66LySAxpG9FmNX6QAQ5Jc8xUWi invoke [1]', <==== LAST REGISTERED PROGRAM IN STACK
  'Program log: YLjF84sCWpQFAAAAAAAAAAUAAABoZWxsbw==',    <==== BELONGS TO PREVIOUS PROGRAM
  'Program 3F3r9Njk8YkVJVyaxW66LySAxpG9FmNX6QAQ5Jc8xUWi consumed 4636 of 200000 compute units',
  'Program 3F3r9Njk8YkVJVyaxW66LySAxpG9FmNX6QAQ5Jc8xUWi success'
]

Whereas Logs for the same transaction initializing an associated account will be:

[
  'Program 8DKf8QXhaptkAxVUEvRYLrwNzfPjAm3GveYrfernAidC invoke [1]',
  'Program 11111111111111111111111111111111 invoke [2]',
  'Program 11111111111111111111111111111111 success',  <==== LAST REGISTERED PROGRAM IN STACK
  'Program log: YLjF84sCWpQFAAAAAAAAAAUAAABoZWxsbw==',   <==== BELONGS TO 8DKf8QX... 
  'Program 8DKf8QXhaptkAxVUEvRYLrwNzfPjAm3GveYrfernAidC consumed 15929 of 200000 compute units',
  'Program 8DKf8QXhaptkAxVUEvRYLrwNzfPjAm3GveYrfernAidC success'
]

Because the code first checks whether execution.program() === this.programId.toString() before triggering an event. The call is never made as this condition can never be met.

The fix pops the last program in the stack if any program ends successfully.
A new assumption being made is that a success will always "belong" to the last program in the stack.

ts/src/program/event.ts Outdated Show resolved Hide resolved
@armaniferrante
Copy link
Member

Let's also add a test for this.

@@ -89,18 +87,19 @@ export class EventParser {
private handleSystemLog(log: string): [string | null, boolean] {
// System component.
const logStart = log.split(":")[0];
// Did the program finish executing?
if (
logStart.match(/^Program (.*) success/g) !== null
Copy link
Contributor Author

@Standaa Standaa Jun 25, 2021

Choose a reason for hiding this comment

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

Following your discord comment here:

It appears the problem is that the previous code makes the assumption that a consumed log will be emitted at the end of cpi.
With the system program, this appears not to be the case.

I removed the "consumed" check as the OR condition crashed the existing test.
Monitoring the "success" log should be enough, as It looks like a "success" log will always be emitted, no matter if the call is a CPI or a trad call. Or maybe there was a reason why you did not do it ?

ctx: Context<TestSimulateAssociatedAccount>,
data: u32,
) -> ProgramResult {
let associated_account = *ctx.accounts.my_account.to_account_info().key;
Copy link
Contributor Author

@Standaa Standaa Jun 25, 2021

Choose a reason for hiding this comment

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

Was not sure how to test creation of the associated account. Tried to check in the client whether the account had been created like so:

const account = await program.account.testData.associated(
  program.provider.wallet.publicKey,
);

But it did not work. My guess is that program.simulate() has a limited lifetime ?
I am passing the pubkey back in an event and checking that it matches as a workaround, but it is not great.

@Standaa Standaa requested a review from armaniferrante June 25, 2021 11:13
@xjcaa
Copy link
Contributor

xjcaa commented Jun 26, 2021

This issue seems to occur when using associated accounts in general, thanks for this :D

@Standaa Standaa marked this pull request as ready for review June 29, 2021 10:36
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.

3 participants