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

chore: simplify bitcoin header validation #3257

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mihailjianu1
Copy link
Contributor

@mihailjianu1 mihailjianu1 commented Dec 19, 2024

This PR removes all checks regarding the timestamp of the header in the bitcoin adapter.

This is meant to simplify the header validation itself, as the checks are anyway made in the bitcoin canister.

The main check that remains is the PoW check, which is by far the hardest to "abuse".

Note that checks such as:

  • has this header already been seen
  • Is the "previous header" not in the store

are already made in the adapter.

@mihailjianu1 mihailjianu1 changed the title Chore: simplify bitcoin header validation chore: simplify bitcoin header validation Dec 19, 2024
@github-actions github-actions bot added the chore label Dec 19, 2024
@mihailjianu1 mihailjianu1 marked this pull request as ready for review December 19, 2024 11:20
@mihailjianu1 mihailjianu1 requested review from a team as code owners December 19, 2024 11:20
@rumenov rumenov requested a review from THLO December 19, 2024 14:26
/// Validates if a header's timestamp is valid.
/// Bitcoin Protocol Rules wiki https://en.bitcoin.it/wiki/Protocol_rules says,
/// "Reject if timestamp is the median time of the last 11 blocks or before"
fn is_timestamp_valid(store: &impl HeaderStore, header: &BlockHeader) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

This is the only check I am a bit hesitant to keep since it is documented as part of the BTC protocol rules

https://en.bitcoin.it/wiki/Protocol_rules

Do you know if testnet and testnet4 follow this particular check ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants