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

Refactor Gravity nonce implementation #1636

Closed
7 tasks done
byte-bandit opened this issue May 30, 2024 · 2 comments
Closed
7 tasks done

Refactor Gravity nonce implementation #1636

byte-bandit opened this issue May 30, 2024 · 2 comments
Assignees
Labels

Comments

@byte-bandit
Copy link

byte-bandit commented May 30, 2024

Problem description

The original gravity code assumes a 1:1 relation between the local chain and the remote chain. Paloma, however, supports tokens across n chains, leaving us with a 1:n relationship.

This causes headaches across the following areas:

  • Paloma only keeps track of the latest observed nonce and latest observed block height for one chain, instead of n
  • Paloma also keeps all attestations in one store, instead of one store per remote chain
  • Attestation assumes that the observed nonces will always increase exactly by 1, which they won't if they're shared between different chains
  • Probably more that I don't see right now

This is the price we pay for not having written the module ourselves and just copied it over. To support our use case, I think we'd need one keeper per remote chain, but I don't think that's feasible with the Cosmos architecture. So instead, we need to hack what we have to support the n chain constraint.

Action plan

  • Refactor Paloma gravity keeper to respect the contextual reference chain ID
    • Last observed BH by chain
    • Last observed nonce by chain
    • Attestations by chain
    • Last nonce by validator and chain
  • Update Compass, event_id actually seems to be the gravity_nonce, but it's used throughout the board. Remove usages across other, non-gravity related functions and revert the last PR that added the second nonce. Also rename it to be more clear on intentions.
  • Update Paloma to work with the new compass, both payload & generative logic. We will keep a separate event_id for better traceability.
@byte-bandit byte-bandit self-assigned this May 30, 2024
byte-bandit added a commit to palomachain/paloma that referenced this issue Jun 3, 2024
# Related Github tickets

- VolumeFi#1636

# Background

This change adds Gravity support for more than one remote chain by
contextualising stored data on Paloma with a chain reference ID. It also
adds the needed changes to the genesis as well as CLI commands.

# Testing completed

- [x] test coverage exists or has been added/updated
- [x] tested in a private testnet

# Breaking changes

- [x] I have checked my code for breaking changes
- [x] If there are breaking changes, there is a supporting migration.
@taariq
Copy link
Member

taariq commented Jun 10, 2024

@byte-bandit this is fixed with palomachain#1174 yes?

@byte-bandit
Copy link
Author

Not completely. The last changes for this I"m still working on.

byte-bandit added a commit to palomachain/paloma that referenced this issue Jun 11, 2024
# Related Github tickets

- VolumeFi#1636
- VolumeFi#842
- VolumeFi#1378
- VolumeFi#1617

# Background

This change addresses quite a lot of ongoing issues on the gravity
project, and the merge will result in a fully functional bridge
implementation across multiple chains and tokens, with full support for
funds exchange.

Rollout will require an updated version of compass with this change
VolumeFi/paloma-compass-evm@95b856d

# Testing completed

- [x] test coverage exists or has been added/updated
- [x] tested in a private testnet

# Breaking changes

- [x] I have checked my code for breaking changes
- [x] If there are breaking changes, there is a supporting migration.

---------

Co-authored-by: Luis Carvalho <[email protected]>
Co-authored-by: Taariq Lewis <[email protected]>
@verabehr verabehr closed this as completed Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants