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

[SUBGRAPH] superTokenLogicCreatedEvents missing recent events #1912

Open
d10r opened this issue Mar 25, 2024 · 3 comments
Open

[SUBGRAPH] superTokenLogicCreatedEvents missing recent events #1912

d10r opened this issue Mar 25, 2024 · 3 comments
Assignees
Labels
Project: SUBGRAPH Superfluid protocol subgraph Type: Bug Something isn't working

Comments

@d10r
Copy link
Collaborator

d10r commented Mar 25, 2024

The script for updating Super Tokens queries superTokenLogicCreatedEvents in order to get a list of previous canonical logic contracts, see https://github.com/superfluid-finance/protocol-monorepo/blob/dev/packages/ethereum-contracts/ops-scripts/gov-upgrade-super-token-logic.js#L166

However this query returns incomplete data, it doesn't include recent deployments.

Example:
On optimism-mainnet, the latest canonical logic contract is 0x78743a68d52c9D6CCF3fF4558f3af510592E3C2D, see factory.

The SG query

query MyQuery {
  superTokenLogicCreatedEvents(orderDirection: desc, orderBy: timestamp) {
    id
    tokenLogic
  }
}

returns 0x882703dc8239e2ba167e06ce1fcf654e17a0bd06 as the most recent logic contract, which was deployed a year ago.

Without having checked, I guess the breakage may be related to switching to Custom Errors or to the restructuring of the SuperTokenFactory (canonical logic now being deployed in a dedicated tx by the script).

@d10r d10r added Project: SUBGRAPH Superfluid protocol subgraph Type: Bug Something isn't working labels Mar 25, 2024
@d10r d10r self-assigned this Sep 24, 2024
@d10r
Copy link
Collaborator Author

d10r commented Sep 24, 2024

network current canonical logic SG last logic SG last event timestamp
Ethereum 0xF0d7d1D47109bA426B9D8A3Cde1941327af1eea3 0xeb69ed9143d33d5fbad67f394456f212c65c1544 1679413211
Polygon 0xfD83982eE75892781242141Ee19E1B42428B8220 0x5ee041478e482ff756a783b8115b06bcecc0fc93 1679412431
Base 0xEB796bdb90fFA0f28255275e16936D25d3418603 N/A N/A
Base Sepolia 0x233a5Bfd65Da07AeB08F2082d2B5B270bc4eA804 N/A N/A

@d10r
Copy link
Collaborator Author

d10r commented Sep 24, 2024

Reason for the missing events: when moving creation of the SuperToken logic out of the SuperTokenFactory (due to contract size limit), we moved emission of the event SuperTokenLogicCreated into the constructor of SuperTokenFactory where it's set as an immutable. Problem with this approach: the event is now emitted by the SuperTokenFactory logic contract, not by the proxy contract where the subgraph expects to see it.

@d10r
Copy link
Collaborator Author

d10r commented Sep 24, 2024

recommendation: remove the event

rationale:
The script gov-upgrade-super-token-logic.js triggering discovery of this bug is the only consumer of the event. Its recently added querying of this event was a small ergonomic improvement, not something really need. What we need is the previous canonical SuperToken logic, which is not difficult to get in other ways. E.g. when preparing gov actions for mainnets with the upgrade action not yet executed, it's even as simple as querying the current canonical ST logic.
Regardless, we shouldn't leave it broken like this, with the subgraph pretending to have the events, but having only old ones.
There's no good solution for fixing it in the subgraph, such that missed events were included.
We could fix it for future events by moving the event emission statement somewhere else in the protocol, e.g. into updateCode. But in order to avoid duplicates when the canonical logic doesn't change between factory upgrades (unlikely, but possible), additional logic and to-be-tested code paths would be created. Unjustified bloat.
Instead it's an opportunity to simplify by removing the event.

If we wanted to cover contract upgrades in the subgraph, a better and more generic approach would be to track the event CodeUpdated for all upgradable contracts. We're currently not doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Project: SUBGRAPH Superfluid protocol subgraph Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants