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

Gravity Rewrite #964

Merged
merged 13 commits into from
Sep 1, 2023
Merged

Conversation

ToasterTheBrave
Copy link
Contributor

@ToasterTheBrave ToasterTheBrave commented Aug 31, 2023

Related Github tickets

Background

A complete rewrite of Gravity to use the functionality from Gravity-Bridge instead of PeggyJV

This strips down the unneeded functionality from Gravity and only includes the parts we need.
I have also removed duplicate functionality for things like valset and validator eth address lookups. We rely on our existing workflows for those.

The new functionality here includes:

  • Vote through governance to track a token as the official token on each supported chain
  • Submit "send-to-eth" transactions to send from a paloma account to an evm account
  • Batching of transactions every 50 blocks
  • Observer "send-to-paloma" events to add to a paloma account when deprecating from an evm account

I'm opening this as draft for now. It's a huge amount of code. The best way to test it is likely to stand it up in the private testnet and verify that everything works as expected.

Testing completed

  • test coverage exists or has been added/updated
  • tested in a private testnet

Breaking changes

  • I have checked my code for breaking changes
  • If there are breaking changes, there is a supporting migration.

Tyler Ruppert added 11 commits July 28, 2023 11:44
* We are only dealing with ugrain, but we have different chains.
  Modifying the request to allow for this, and leaving it a bit hacky
for now.  Will revisit
* We don't care about how profitable a batch is, we want to always allow
  creating a new batch
* When a new batch is requested but there aren't any transactions, just
  do nothing.  This isn't an error, so don't return an error
* Set the gravity salt for signing to be something other than the
  default in genesis
… into one commit

* Remove valset pieces and rely on existing valset in Paloma
* Remove orchestrator setting and rely on EVM implementation for getting eth keys
* Remove lots of other things that existed in Gravity that we don't need, like deploying ERC20s, logic calls, blacklisting, bridge halting.
* Set up the turnstone ID from the smart contract to use for signing
* Fix all the tests that broke from doing all the above
* Add some cli commands to investigate what is going on with batches
* Batch up transactions every 50 blocks
* Assign batches to pigeons using out assignment logic
* Rewrite a lot of code that overly utilized panic().  When we panic on one host, we risk a different app hash
* Allow sending our coin (GRAIN) to any chain.  We keep track of the token on that chain and can send it to any chain that has a token.  This mapping is controlled by governance
* Remove fees.  This will be handled elsewhere
Copy link
Contributor

@byte-bandit byte-bandit left a comment

Choose a reason for hiding this comment

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

Throwing in a blocker for now until we have 1.7.3, I think this should go into 1.8.0.

Copy link
Contributor

@byte-bandit byte-bandit left a comment

Choose a reason for hiding this comment

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

This PR is simply too large to be reviewed sensibly, it's impossible to tell which part are custom adaptions and which is directly taken from the original gravity module.

To be honest, I think we'd be going back and forth for a week or two defining common coding guidelines for the Paloma project and bringing everything up to par. It's something I'd like to do given the time to reduce the clutter from merging in foreign code, but it will completely kill project velocity.

@ToasterTheBrave
Copy link
Contributor Author

I've added the migration to this. The module name needed to be changed to "gravity2" because the "gravity" stores changed so much, and we can't delete and add in the same migration. We can rename the module to "gravity" in a later migration if we choose.

The upgrade path has been tested and works. This can be merged

@taariq taariq marked this pull request as ready for review September 1, 2023 18:17
@taariq taariq merged commit 96698d6 into palomachain:master Sep 1, 2023
1 check passed
@taariq taariq deleted the toaster/gravity-rewrite branch September 1, 2023 18:17
@taariq
Copy link
Contributor

taariq commented Sep 1, 2023

merged

@maharifu maharifu mentioned this pull request Jun 19, 2024
4 tasks
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.

3 participants