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

feat: Pigeon Feed #1262

Merged
merged 16 commits into from
Aug 16, 2024
Merged

feat: Pigeon Feed #1262

merged 16 commits into from
Aug 16, 2024

Conversation

byte-bandit
Copy link
Contributor

Background

Things brings back the changes for Paloma v2.0.0.

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.

@byte-bandit byte-bandit requested a review from maharifu August 16, 2024 15:40
@byte-bandit byte-bandit changed the title Dev16 Bring back dev16 Aug 16, 2024
@byte-bandit byte-bandit changed the title Bring back dev16 feat: Pigeon Feed Aug 16, 2024
byte-bandit and others added 4 commits August 16, 2024 17:01
# Related Github tickets

- VolumeFi#344
- VolumeFi#1761

# Background

> [!IMPORTANT]  
> This will only merged back to master after Pigeon changes and
successful smoke tests on PTN

This change adds the infrastructure for Paloma to support gas
estimation. Most notably, it

- changes the lifecycle of the consensus queue message
- adds endpoints for Pigeons to query for and report gas estimation
- uses an endblocker to process any estimated messages

## Lifecycle changes

The current message lifecycle is very bare bone. Pigeons ask for
messages to be signed, and ultimately relay it. There is little
filtering or state processing done. A message will even be handed out
for relaying long before it has reached consensus on signatures (Pigeon
checks that, not Paloma).

This change adds the concept of gas estimation to all consensus queue
messages. It's not super pretty, since really only EVM messages will
need the data. However, the CQ module is the main interface for managing
the queue and message lifecycles. It's already grossly interwoven with
the EVM module, and the whole idea of abstraction has not really paid
off. We hope to address this in the future.

Paloma will hand out messages for gas estimation once they have reached
consensus on signatures. Pigeons will not relay a message that requires
gas estimation and has not yet received one. Even though a message might
have been fully signed, it won't be relayed until gas estimation is
completed.

For this, Pigeons will query the message, build the payload and estimate
the gas using their configured RPC provider. They will send their
estimate to Paloma, and Paloma will start tracking every estimate. We
expect estimates to be different across RPC providers, and have settled
to use the median value of all collected estimates, with a generous
`1.2` multiplier for safety margins.

Paloma will continuously monitor messages and check whether they have
received enough estimates to calculate the estimate. IF the message
happens to be an SLC message, Paloma will then proceed to automatically
calculated the fees based on the relayer fee settings for this message.

Once a message has been estimated, all signatures are removed from the
message and the signing process starts again, this time using the
embedded information (fees & estimates). After this second round of
signatures, the messages will finally be sent.

# Testing completed

- [x] test coverage exists or has been added/updated
- [ ] 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]>
# Related Github tickets

- VolumeFi#1761
- VolumeFi#344

# Background

This change adds the needed building bricks for gas estimation to the
Skyway module, which works independently from the consensus module and
regular relay messages. Ideally, the Skyway stuff should be boiled down
to only perform some additional logic on top of the CQ, but for now, we
will have to live with this solution.

# Testing completed

- [x] test coverage exists or has been added/updated
- [ ] 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.
# 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.
# Background

The fee manager smart contract will be independent from compass and the
bridge. It will need to be deployed manually before compass, and
ownership will have to be passed to compass after its deployment.

> [!CAUTION]
> There is no support yet for automatic compass upgrades, as they will
require a handover of the fee manager just like ERC20 tokens. This will
be added in a follow up MR.

# Testing completed

- [x] test coverage exists or has been added/updated
- [ ] 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.
x/skyway/module.go Dismissed Show dismissed Hide dismissed
maharifu and others added 11 commits August 16, 2024 17:04
- VolumeFi#1891

Even if we don't want to move test tokens, since we have them set,
paloma will still try to move them. This will fail since (new) pigeon
won't be compatible with (old) compass, so we need to disable this
functionality before compass 2.0 is deployed and re-enable it again
afterwards.

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

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

- Closes VolumeFi#1042

# Background

Before issuing a jit valset update, check the queue for existing valset
updates with the same ID.

# 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.
# Background

Small change needed on the contract between Paloma/Pigeon.

# Testing completed

- [x] test coverage exists or has been added/updated
- [ ] 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.
# Testing completed

- [x] test coverage exists or has been added/updated
- [ ] 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.
# Related Github tickets

- VolumeFi#1848

# Background

Reset skyway nonce everytime we redeploy compass.

- When a new compass becomes active on a chain, emit an internal event
- The skyway keeper receives this event and resets the last observed
nonce
- Skyway attestations from the previous compass are ignored

This PR will be followed by a related PR to pigeon

# Testing completed

- [ ] 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.
# Background

Messages will now have different bytes to sign before and after
including gas estimates, so we need `BytesToSign` to be more dynamic.
This change computes `BytesToSign` on demand when needed, so we have the
necessary flexibility.

# Testing completed

- [ ] 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.
# Background

This adds mutable BytesToSign to the skyway module, they get refreshed
after gas estimate is attached to the batch.

# Testing completed

- [x] test coverage exists or has been added/updated
- [ ] 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.
# Background

Include gas estimate and fee structure in turnstone messages hash.

# 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.
# Background

Adapt turnstone message attestation test code to the new compass ABI.

# Testing completed

- [x] test coverage exists or has been added/updated
- [ ] 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: Christian Lohr <[email protected]>
Merge master back to dev16 branch.
# 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]>
maharifu
maharifu previously approved these changes Aug 16, 2024
Copy link
Contributor

@maharifu maharifu left a comment

Choose a reason for hiding this comment

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

Let's do this!

Copy link
Contributor

@maharifu maharifu left a comment

Choose a reason for hiding this comment

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

👍

@byte-bandit byte-bandit enabled auto-merge (squash) August 16, 2024 16:27
@byte-bandit byte-bandit merged commit fca3518 into master Aug 16, 2024
4 checks passed
@byte-bandit byte-bandit deleted the dev16 branch August 16, 2024 16:31
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.

2 participants