Skip to content

Fix skipped handler on contract register event case#304

Merged
JonoPrest merged 4 commits into
mainfrom
jp/fix-dynamic-contract-skipped-handler
Oct 30, 2024
Merged

Fix skipped handler on contract register event case#304
JonoPrest merged 4 commits into
mainfrom
jp/fix-dynamic-contract-skipped-handler

Conversation

@JonoPrest

@JonoPrest JonoPrest commented Oct 29, 2024

Copy link
Copy Markdown
Collaborator

Closes #303

Need to write a test that enforces this case is handled.

Happens in the context of a batch being processed when:

  1. The batch contains a contract registration
  2. an event afterwards runs a contractRegister function that registers no contracts
  3. Does not add the given event to the unprocessed events (which should have ended up on the arbitrary events queue)

@JonoPrest JonoPrest requested a review from DZakh October 29, 2024 15:58
}
})

Async.it("One registration event with dynamicContracts, one without", async () => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's only this case that was broken before hand. So it's not super critical.

@JonoPrest

Copy link
Copy Markdown
Collaborator Author

To expand, this is not super critical. The indexers that it affects is when you have a "contractRegister" and a "handler" function for a given event.

The contracts would have still been registered as expected. It's just that the event would never be added to the unprocessed batch so it would never get a chance to run the handler.

@JonoPrest JonoPrest force-pushed the jp/fix-dynamic-contract-skipped-handler branch from 5a6d1c8 to c08e8a0 Compare October 30, 2024 13:49

let val = switch (dynamicContracts, dynamicContractRegistrations) {
| ([], dynamicContractRegistrations) => dynamicContractRegistrations
| ([], None) => None

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

To be clear, it was hitting this case when the contractRegister function produced no registrations (possibly because of duplicate registration)

AND the current dynamicContratRegistrations was Some. Then it would just pass through the current registrations without adding itself to the unprocessed batch. Which we only want on the None case.

@DZakh DZakh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch 👍

@JonoPrest JonoPrest force-pushed the jp/fix-dynamic-contract-skipped-handler branch from 8fc968a to 4eee8ca Compare October 30, 2024 14:50
@JonoPrest JonoPrest enabled auto-merge (squash) October 30, 2024 14:50
@JonoPrest JonoPrest merged commit 7060a46 into main Oct 30, 2024
@JonoPrest JonoPrest deleted the jp/fix-dynamic-contract-skipped-handler branch October 30, 2024 14:55
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.

Dynamic contract preregistration true 👍 and false 🐛 has different outcomes

2 participants