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: remove retry limit from erc20 transfers #1214

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

byte-bandit
Copy link
Contributor

Related Github tickets

Background

This change will make sure that ERC20 transfers are being retried continuously instead of stopping after 2 retries to try and attempt to prevent a deadlock. There might still be a need for a manual unlock down the road.

Additionally, we now make sure to stop processing skyway messages while we have an ongoing contract deployment, to avoid a desync of the nonces on the old and new compass.

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 July 5, 2024 15:48
@byte-bandit
Copy link
Contributor Author

I ultimately decided against the watch dog approach, since we're lacking access to a lot of dynamic information at the EndBlocker, that is naturally available during the attestation process. So instead, we just keep brute forcing the message.

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.

Nice one! LGTM 👍

@taariq
Copy link
Contributor

taariq commented Jul 5, 2024

@byte-bandit needs a signed commit to merge.

@byte-bandit byte-bandit merged commit ea0d332 into palomachain:master Jul 5, 2024
4 checks passed
@byte-bandit byte-bandit deleted the feature/compass-deployment branch July 5, 2024 16:48
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