-
Notifications
You must be signed in to change notification settings - Fork 35.6k
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
Restrict timestamp when mining a diff-adjustment block to prev-600 #15481
Conversation
0cc366b
to
93359e5
Compare
Concept ACK. Travis failure looks like some kind of timeout. |
93359e5
to
1d72dd8
Compare
def run_test(self): | ||
self.log.info('Create some old blocks') | ||
block_1 = self.nodes[0].generate(1)[0] | ||
for i in range(2013): |
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.
Nit: for _ in range(2013)
is more idiomatic when the index is unused :-)
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.
Any reason not to generate all blocks with a singe call here: self.nodes[0].generate(nblocks=2013)
?
1d72dd8
to
bdb4372
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Travis failures are wholly unrelated to this PR, it seems. |
assert_equal(mining_info['currentblocktx'], 0) | ||
assert_equal(mining_info['currentblockweight'], 4000) | ||
|
||
self.restart_node(0) |
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.
Why restart node here? Is this just to reset mocktime? Can you call setmocktime(0)
?
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.
Its just easier than trying to figure out what the current time is and then make sure that its correct?
This prepares us for a potential future timewarp-fixing softfork by ensuring that we always refuse to mine blocks which refuse to exploit timewarp, no matter the behavior of other miners. Note that we allow timestamp to go backwards by 600 seconds on the difficulty-adjustment blocks to avoid bricking any existing hardware which relies on the ability to roll nTime by up to 600 seconds. Note that it is possible that the eventual softfork to fix timewarp has a looser resetriction than the 600 seconds enforced here, however it seems unlikely we will apply a tighter one, and its fine if we restrict things further on the mining end than in consensus.
bdb4372
to
584a7d4
Compare
I haven't worked on any consensus cleanup logic in some time, and it seems unlikely to be picked back up. Until there's a direction there, this probably doesn't make sense. |
This prepares us for a potential future timewarp-fixing softfork by
ensuring that we always refuse to mine blocks which refuse to
exploit timewarp, no matter the behavior of other miners. Note that
we allow timestamp to go backwards by 600 seconds on the
difficulty-adjustment blocks to avoid bricking any existing
hardware which relies on the ability to roll nTime by up to 600
seconds.
Note that it is possible that the eventual softfork to fix timewarp
has a looser resetriction than the 600 seconds enforced here,
however it seems unlikely we will apply a tighter one, and its fine
if we restrict things further on the mining end than in consensus.