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

[Hardening] Have one atomic function call on compass for all handovers #1960

Closed
byte-bandit opened this issue Aug 8, 2024 · 2 comments
Closed

Comments

@byte-bandit
Copy link

byte-bandit commented Aug 8, 2024

Instead of making n calls to change ownership of ERC20 tokens and fee manager and potentially ending in a situation where some calls went through and other don't, it would be faster and more efficient to have only one function on compass to call, that forwards handover calls to all tokens and the fee manage.

Since those will all be in the same TX, it will either go through or fail.

@byte-bandit byte-bandit self-assigned this Aug 8, 2024
@taariq taariq added this to the v1.16.0 milestone Aug 8, 2024
@byte-bandit
Copy link
Author

@webelf101 Here's the suggested implementation:

  1. Make update_compass_address_in_fee_manager an internal function.
  2. Add a new public function compass_handover()

That new function should:

  • Verify the call was made from Paloma and assert consensus
  • Accept a batch of submit logic call arguments
  • For each item in the batch, execute the submit logic call (ignoring fees)
  • Lastly, run update_compass_address_in_fee_manager

That way, when we upgrade compass, we can call the old compass once. The submit logic call batch would contain the calls to new_compass() for each deployed ERC20 token. That way, we can make sure that ALL dependencies will be updated in one atomic transaction.

This would be a great improvement on the current situation, where we have to make n calls instead of one, and deployments can get stuck if one or more tokens fail to be updated.

@webelf101
Copy link

VolumeFi/paloma-compass-evm@ee37fff
Can this solve this issue? @byte-bandit

@byte-bandit byte-bandit modified the milestones: v2.0.0, v2.0.1 Aug 16, 2024
@byte-bandit byte-bandit modified the milestones: v2.1.0, v2.1.1, v2.1.2, next Aug 23, 2024
taariq pushed a commit to palomachain/paloma that referenced this issue Aug 29, 2024
# Related Github tickets

- VolumeFi#1960
- VolumeFi#1951
- VolumeFi#1956
- VolumeFi#2043

# Background

This change makes use of the atomic handover endpoint on compass,
re-enables the token relink and ensures ownership of fee manager is
transferred as well.

# 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.
byte-bandit added a commit to palomachain/paloma that referenced this issue Aug 30, 2024
# Related Github tickets

- VolumeFi#1960
- VolumeFi#1951
- VolumeFi#1956
- VolumeFi#2043

# Background

This change makes use of the atomic handover endpoint on compass,
re-enables the token relink and ensures ownership of fee manager is
transferred as well.

# 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.
@byte-bandit byte-bandit modified the milestones: next, v2.1.2 Aug 30, 2024
@byte-bandit byte-bandit modified the milestones: v2.1.2, v2.1.3 Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants