rpc: change setmocktime check to use IsMockableChain #18263

pull glozow wants to merge 1 commits into bitcoin:master from glozow:rpc-setmocktime-check changing 1 files +3 −2

glozow commented at 10:25 PM on March 4, 2020: member

Change: Update the if statement in setmocktime to use IsMockableChain chainparams function (aka m_is_mockable_chain) instead of MineBlocksOnDemand

Rationale: It's a more appropriate check for whether or not chain is in RegTest, as discussed in #18037
glozow renamed this:
~~rpc change setmocktime check to use IsMockableChain~~
[WIP] rpc change setmocktime check to use IsMockableChain
on Mar 4, 2020
glozow renamed this:
~~[WIP] rpc change setmocktime check to use IsMockableChain~~
[WIP] [rpc] change setmocktime check to use IsMockableChain
on Mar 4, 2020
fanquake added the label RPC/REST/ZMQ on Mar 4, 2020
glozow renamed this:
~~[WIP] [rpc] change setmocktime check to use IsMockableChain~~
[WIP] rpc: change setmocktime check to use IsMockableChain
on Mar 4, 2020
glozow renamed this:
~~[WIP] rpc: change setmocktime check to use IsMockableChain~~
rpc: change setmocktime check to use IsMockableChain
on Mar 4, 2020
amitiuttarwar commented at 11:17 PM on March 4, 2020: contributor

ACK 4264e97b42e87ff1e103eb1f919cff4c0f56c4fe. Thanks for this follow up :)
glozow marked this as ready for review on Mar 4, 2020

in src/rpc/misc.cpp:348 in 4264e97b42 outdated

 344 | @@ -345,7 +345,7 @@ static UniValue setmocktime(const JSONRPCRequest& request)
 345 |                  RPCExamples{""},
 346 |              }.Check(request);
 347 |  
 348 | -    if (!Params().MineBlocksOnDemand())
 349 | +    if (!Params().IsMockableChain())

MarcoFalke commented at 1:45 PM on March 5, 2020:

    if (!Params().IsMockableChain()) {

Might as well fix up with the new style (opening and closing {}). Not a blocking nit.

glozow commented at 5:57 PM on March 5, 2020:

done e8600d9

MarcoFalke approved
MarcoFalke commented at 1:45 PM on March 5, 2020: member

ACK
in src/rpc/misc.cpp:349 in 4264e97b42 outdated
```
 344 | @@ -345,7 +345,7 @@ static UniValue setmocktime(const JSONRPCRequest& request)
 345 |                  RPCExamples{""},
 346 |              }.Check(request);
 347 |  
 348 | -    if (!Params().MineBlocksOnDemand())
 349 | +    if (!Params().IsMockableChain())
 350 |          throw std::runtime_error("setmocktime for regression testing (-regtest mode) only");
```
jonatack commented at 4:03 PM on March 5, 2020:

Non-blocking nit: if you retouch this to add the opening and closing curly braces, could you also add the missing "is" in the error message? Like in https://github.com/bitcoin/bitcoin/pull/18037/files#diff-ad6efdc354b57bd1fa29fc3abb6e2872R383. Happy to re-ACK.

glozow commented at 5:57 PM on March 5, 2020:

done e8600d9
jonatack commented at 4:04 PM on March 5, 2020: member

ACK 4264e97b42e87ff1e103eb1f919cff4c0f56c4fe modulo nits
promag commented at 4:30 PM on March 5, 2020: member

ACK 4264e97b42e87ff1e103eb1f919cff4c0f56c4fe.
MarcoFalke commented at 6:05 PM on March 5, 2020: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
[rpc] changed MineBlocksOnDemand to IsMockableChain 2455aa5d7f
glozow force-pushed on Mar 5, 2020
glozow requested review from jonatack on Mar 5, 2020
jonatack commented at 6:32 PM on March 5, 2020: member

ACK 2455aa5d7f54befeade05795ed8f5dd89d01042a

Thanks @gzhao408!

MarcoFalke commented at 6:39 PM on March 5, 2020: member

ACK 2455aa5d7f54befeade05795ed8f5dd89d01042a 🙇

<details><summary>Show signature and timestamp</summary>

Signature:

Timestamp of file with hash 1b061df7687ff438c7b5c195a12c2d0b089b1ff6cc7d010071513fbb19807002 -

</details>

MarcoFalke merged this on Mar 5, 2020
MarcoFalke closed this on Mar 5, 2020
glozow deleted the branch on May 25, 2020
Fabcien referenced this in commit aa3561df33 on Jan 5, 2021
PastaPastaPasta referenced this in commit 023cda03c2 on Jun 27, 2021
PastaPastaPasta referenced this in commit 653269009c on Jun 28, 2021
PastaPastaPasta referenced this in commit 98b287fb52 on Jun 29, 2021
MarcoFalke locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#18037 Util: Allow scheduler to be mocked