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

experimental: advance-runner does not close epochs when claims stream has events from a different dapp address #522

Closed
endersonmaia opened this issue Jul 17, 2024 · 0 comments · Fixed by #551
Milestone

Comments

@endersonmaia
Copy link
Contributor

🙂 Expected behavior

When using rollups-node with CARTESI_EXPERIMENTAL_SUNODO_VALIDATOR_ENABLED = 'true' and defining CARTESI_EXPERIMENTAL_SUNODO_VALIDATOR_REDIS_ENDPOINT so that you can run the cartesi-rollups-authority-claimer outside the supervision of the rollups-node go supervisor pointing to the same redis isntance..

It's expected that one can start another rollups-node instance for another application with the same setup.

At the end, multiple rollups-node will feed the {chain-XXX}:rollups-claims and a single claimer configured to make claims for that XXX chain should be able to make all that claims.

🫠 Actual behavior

In the setup described above, the advance-runner for a specific rollups-node application instance can't append the stream if there's already some entries for another application.

🧪 Minimal test case

I don't have an automated test, but have evidence from the logs and some dumps from redis.

The following dump, is from the rollups-claimer stream with a previous version of the bug-buster application.

When the stream had this content, the new version of the bug-buster (another contract address), the advance-runner logged the message produced claim but the stream never was appended with the claim.

XRANGE "{chain-11155420}:rollups-claims" - +
1) 1) "1719522678183-0"
   2) 1) "payload"
      2) "{\"dapp_address\":\"548fce869928a92f82a1f3fa1e6a918d44d8abdc\",\"epoch_index\":0,\"epoch_hash\":\"b8e1f606fa583b58dde40dd2fc557876621f801746108ecee3253766439f9beb\",\"first_index\":0,\"last_index\":9}"
2) 1) "1719522684214-0"
   2) 1) "payload"
      2) "{\"dapp_address\":\"548fce869928a92f82a1f3fa1e6a918d44d8abdc\",\"epoch_index\":1,\"epoch_hash\":\"dde3a812ed12891d0bd906b3005b2955969b38f9b497dd29c9631243f006b09a\",\"first_index\":10,\"last_index\":15}"
3) 1) "1719522690236-0"
   2) 1) "payload"
      2) "{\"dapp_address\":\"548fce869928a92f82a1f3fa1e6a918d44d8abdc\",\"epoch_index\":2,\"epoch_hash\":\"96a7e989fd4cf739f9334945883ef9e5ec89dd280b49426081a590cd134792e6\",\"first_index\":16,\"last_index\":18}"
4) 1) "1719522694257-0"
   2) 1) "payload"
      2) "{\"dapp_address\":\"548fce869928a92f82a1f3fa1e6a918d44d8abdc\",\"epoch_index\":3,\"epoch_hash\":\"e75419bf3ef5a098fc57c3a4143393943a738e9d56a1488f425ee02aeb88b68d\",\"first_index\":19,\"last_index\":21}"

After deleting the stream, advance-runner was able to create the stream and finally produce the claim entries into the rollups-claims stream.

🌎 Environment

This was observed with rollups-node:v1.5.0-rc1.

✔️ Possible solutions

A

We could work with a stream for each $chain-$dapp pair, as we do with inputs and outputs streams.

That would probably involve multiple components, like advance-runner and authority-claimer, at least.

  • {chain-xxx-app-yyy}:rollups-inputs
  • {chain-xxx-app-yyy}:rollups-outptus
  • {chain-xxx-app-yyy}:rollups-claims

B

We could make advance-runner aware of multiple applications when appending thev stream.

Probably, this would need to change only advance-runner internal logic.

A Redis Stream, is an append only data structure, and it would be necessary to navigate all the stream elements to know if we should add a claim entry or not.

Maybe we could leverage Consumer Groups, but I'm not sure if that's possible, and we'd need to investigate.

References:

@endersonmaia endersonmaia added the bug Something isn't working label Jul 17, 2024
@endersonmaia endersonmaia changed the title advance-runner can't procude claims into the rollups-claims Redis stream if claims for other app is already there advance-runner can't produce claims into the rollups-claims Redis stream if claims for other app is already there Jul 17, 2024
@marcelstanley marcelstanley changed the title advance-runner can't produce claims into the rollups-claims Redis stream if claims for other app is already there experimental: claimer does not close epochs if redis stream is not empty Jul 17, 2024
@marcelstanley marcelstanley changed the title experimental: claimer does not close epochs if redis stream is not empty experimental: advance runner does not close epochs when claims stream has events from a different dapp address Jul 17, 2024
@marcelstanley marcelstanley changed the title experimental: advance runner does not close epochs when claims stream has events from a different dapp address experimental: advance-runner does not close epochs when claims stream has events from a different dapp address Jul 17, 2024
@marcelstanley marcelstanley self-assigned this Jul 17, 2024
@marcelstanley marcelstanley moved this to 🏗 In progress in Node Unit Jul 17, 2024
@marcelstanley marcelstanley moved this from 🏗 In progress to 📋 Backlog in Node Unit Jul 18, 2024
@marcelstanley marcelstanley modified the milestones: TBD, 1.5.1 Jul 26, 2024
@marcelstanley marcelstanley added the triage To be triaged label Jul 29, 2024
@marcelstanley marcelstanley removed the triage To be triaged label Jul 29, 2024
@marcelstanley marcelstanley modified the milestones: 1.5.1, TBD Jul 30, 2024
@marcelstanley marcelstanley removed the bug Something isn't working label Aug 7, 2024
@marcelstanley marcelstanley modified the milestones: TBD, 1.5.1 Aug 7, 2024
@marcelstanley marcelstanley removed the status in Node Unit Aug 8, 2024
@marcelstanley marcelstanley moved this to 👀 In review in Node Unit Aug 9, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Node Unit Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants