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

fix: calculate the current median time from tip #878

Merged
merged 2 commits into from
May 29, 2019

Conversation

keroro520
Copy link
Contributor

@keroro520 keroro520 commented May 22, 2019

BREAKING CHANGE: Original implementation use [Tip-BlockMedianCount .. Tip-1] to calculate the current block median time. According to the notion of BlockMedianTime in bip-0113 , here change to use [Tip-BlockMedianCount+1 .. Tip] instead.

@keroro520 keroro520 requested a review from a team May 22, 2019 13:31
nervos-bot[bot]
nervos-bot bot previously requested changes May 22, 2019
Copy link

@nervos-bot nervos-bot bot left a comment

Choose a reason for hiding this comment

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

Hold as requested by @keroro520.

@nervos-bot
Copy link

nervos-bot bot commented May 22, 2019

@TheWaWaR is assigned as the leading reviewer

@keroro520
Copy link
Contributor Author

Block by #854

@doitian
Copy link
Member

doitian commented May 23, 2019

@nervos-bot-user ci status e3e203a
CI: success

Sync status

@doitian
Copy link
Member

doitian commented May 23, 2019

@nervos-bot-user ci status e3e203a
CI: success

Sync status, again.

traits/src/block_median_time_context.rs Outdated Show resolved Hide resolved
@keroro520 keroro520 force-pushed the fix-current-median-time branch 2 times, most recently from 545dcd0 to 2884673 Compare May 24, 2019 07:13
@keroro520 keroro520 requested a review from quake May 24, 2019 07:13
@keroro520 keroro520 requested a review from TheWaWaR May 24, 2019 07:17
@keroro520 keroro520 changed the title [HOLD] Fix current median time Fix current median time May 24, 2019
@nervos-bot nervos-bot bot dismissed their stale review May 24, 2019 07:17

Unhold as requested by @keroro520.

@keroro520
Copy link
Contributor Author

Block by #854

#854 has been merged. This PR is ready.
I add some tests about valid_since. PTAL @quake

@keroro520 keroro520 changed the title Fix current median time fix: calculate the current median time from tip May 27, 2019
@keroro520 keroro520 added the breaking change The feature breaks consensus, database, message schema or RPC interface. label May 27, 2019
Original implementation use [Tip-BlockMedianCount .. Tip-1] to calculate the
current block median time. According to the notion of BlockMedianTime in
https://github.com/bitcoin/bips/blob/master/bip-0113.mediawiki#specification ,
here change to use [Tip-BlockMedianCount+1 .. Tip] instead.

pub fn since_from_absolute_block_number(block_number: BlockNumber) -> u64 {
FLAG_SINCE_BLOCK_NUMBER | block_number
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think those functions should add to enum SinceMetric and should use in send_transaction RPC in the future.

@doitian doitian merged commit 1f22471 into nervosnetwork:develop May 29, 2019
@doitian doitian added the b:consensus Break consensus label May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:consensus Break consensus breaking change The feature breaks consensus, database, message schema or RPC interface.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants