-
Notifications
You must be signed in to change notification settings - Fork 405
Add median-time-past (MTP) calculation to CheckPoint #2037
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
base: master
Are you sure you want to change the base?
Conversation
Add MTP calculation methods for CheckPoint following Bitcoin's BIP113: - `median_time_past()`: calculates MTP for current block using previous 11 blocks - `next_median_time_past()`: calculates MTP for next block using current + previous 10 blocks - Returns None when timestamps unavailable or required blocks missing Includes comprehensive tests for all edge cases. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
f97553f
to
0f75c22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cACK 0f75c22
I'll just give it a try with some other test scenarios before final ACK.
f97553f
to
46e751f
Compare
pub fn median_time_past(&self) -> Option<u32> { | ||
let current_height = self.height(); | ||
let earliest_height = current_height.saturating_sub(Self::MTP_BLOCK_COUNT); | ||
self._median_time_past(earliest_height..current_height) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious if these new APIs could output wrong/invalid values if the blocks inserted have a wrong/invalid time field e.g malicious block source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely. We are trusting the chain source here (and everywhere in BDK). Eventually, we should think about how to verify all consensus rules in BDK: difficulty, PoW, MTP, etc.
46e751f
to
74d30d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cACK 74d30d8
I didn't really find other test vectors that we could use.
Another thing that should probably be handled in a future PR is updating the calculation of Balance for the immature category, we probably should update it to consider transactions that are not yet mature considering the Locktime and MTP for it.
Description
This PR adds the ability to calculate median-time-past (MTP) for
CheckPoint
structures, implementing the functionality described in #2036.MTP is a critical component of Bitcoin's consensus rules (BIP113) used for time-based transaction validations. It provides a more robust timestamp by using the pseudo-median of up to 11 previous block timestamps.
Notes to the reviewers
The MTP calculation needs double checking.
CheckPoint::median_time_past
calculates the MTP value by looking at the 11 blocks that are BEFORE the current. If we call.median_time_past
on the tip, is this the right value to compare against timelocked transactions going into the mempool?Changelog notice
Added
block_time()
method toToBlockHash
trait to optionally return block timestampsmedian_time_past()
andnext_median_time_past()
methods toCheckPoint
for calculating MTP according to BIP113Checklists
All Submissions:
cargo fmt
andcargo clippy
before committingNew Features:
Fixes #2036
🤖 Generated with Claude Code