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

alpha: Rollups v2 support #52

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

brunomenezes
Copy link
Collaborator

@brunomenezes brunomenezes commented Nov 4, 2024

Summary

Code changes to support both rollup contracts v1 and v2 (currently available only for devnet and Sepolia).

Changes:

  • Added new property to the Application model to be queryable based on rollups version without the need to check the factory address (Depending on the query, it means no join).
  • Added new handlers for ApplicationCreated and InputAdded events.
  • The existing handler for OwnershipTransferred was updated to deal with both v1 and v2 events instead of adding a new one, as the topic hash for both contracts is the same. After looking at the generated code from ABI, I believe both handlers would be called sequentially, deeming a new handler unnecessary. PS: I assume it will be picked as the topic matches. There are at least three apps created on Sepolia, but none emitted the event.
  • The config function was updated, adding the addresses for v2, including v2 information when it is supported in the network (currently only Devnet and Sepolia).
  • Updated logic for preloading application address to include v2 factory.
  • Preloaded content was also bumped for Mainnet and Sepolia.

Events

// CartesiApplication
OwnershipTransferred: event("0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0", "OwnershipTransferred(address,address)", {"previousOwner": indexed(p.address), "newOwner": indexed(p.address)}),

// CartesiDApp
OwnershipTransferred: event("0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0", "OwnershipTransferred(address,address)", {"previousOwner": indexed(p.address), "newOwner": indexed(p.address)}),

@brunomenezes brunomenezes self-assigned this Nov 4, 2024
@brunomenezes brunomenezes requested review from tuler and nevendyulgerov and removed request for tuler and nevendyulgerov November 14, 2024 01:06
@brunomenezes brunomenezes reopened this Nov 14, 2024
@brunomenezes brunomenezes requested a review from tuler November 14, 2024 02:40
@brunomenezes brunomenezes marked this pull request as ready for review November 14, 2024 02:40
@brunomenezes
Copy link
Collaborator Author

brunomenezes commented Nov 14, 2024

Hey @tuler, could you confirm my bullet point 3 about the OwnershipTransferred? Because as for this handler is always about the topic comparison I believe having two handlers even though using the events generated by different ABIs, it would still have one Log go through the v1 handler then the new-handler doing unnecessary operations.

@brunomenezes brunomenezes force-pushed the alpha/rollups-v2-support branch from 98000e3 to f04be0e Compare November 15, 2024 03:03
@brunomenezes brunomenezes temporarily deployed to cartesiscan-api-v2-worker November 20, 2024 06:46 Inactive
@brunomenezes brunomenezes force-pushed the alpha/rollups-v2-support branch from f04be0e to cd50c6d Compare December 9, 2024 03:55
@brunomenezes brunomenezes added the Status: On-Hold On-hold for further development, because upcoming changes will affect the Issue or PR targeted label Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: On-Hold On-hold for further development, because upcoming changes will affect the Issue or PR targeted
Projects
Status: 👁 In Review
Development

Successfully merging this pull request may close these issues.

1 participant