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.

Sign up for GitHub

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

Jump to bottom

Usage of longest-chain instead of most-work #1186

Open
gregdhill opened this issue Sep 1, 2023 · 2 comments · May be fixed by #1198
Open

Usage of longest-chain instead of most-work #1186

gregdhill opened this issue Sep 1, 2023 · 2 comments · May be fixed by #1198
Assignees
@nakul1010
Labels
bug Something isn't working prio:high

Comments

@gregdhill
Copy link
Member

Describe the bug
The BTCRelay incorrectly sets the main chain as the longest fork instead of that which has the most work. This means that an attacker with significant mining power could mine a chain with an invalid timestamp to lower the difficulty after each readjustment period. It is currently not economically feasible to exploit due to the required mining requirements.

Expected behavior
We should only reorg when the fork has more work than the current main chain, see how electrum does that here.

Additional context
The summa relay here only checks the chain work when reorging but it does not support reorgs over 2017 blocks.
@gregdhill gregdhill added bug Something isn't working prio:high labels Sep 1, 2023
@nud3l nud3l added this to Backlog Sep 1, 2023
@github-project-automation github-project-automation bot moved this to New 🆕 in Backlog Sep 1, 2023
@nakul1010
Copy link
Member

nakul1010 commented Sep 6, 2023 •
edited
Loading

@gregdhill Thanks for the references few questions

  • I am expected to extend this if condition
    • &&( get_chainwork(current_fork.max_height) >= get_chainwork(prev_chain.max_height))
  • Regarding the target var in electrum is same as in bitcoin types here right ?

@gregdhill
Copy link
Member Author

Yes we need to extend that condition and we should also update ensure_no_ongoing_fork. I can't see which target type you are linking to in electrum, but I am assuming it is the same.

@nakul1010 nakul1010 linked a pull request Sep 20, 2023 that will close this issue
Open
4 tasks
@gregdhill gregdhill mentioned this issue Oct 10, 2023
Closed
12 tasks
@gregdhill gregdhill mentioned this issue Jul 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nakul1010 nakul1010

Labels
bug Something isn't working prio:high
Projects
Backlog
Status: New 🆕
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Fix BTCRelay Chain Reorganization to Prioritize Work Over Length interlay/interbtc
2 participants
@gregdhill @nakul1010