Minor typo cleanup #3300

pull lian wants to merge 1 commits into bitcoin:master from lian:minor_typo_cleanup changing 1 files +3 −3

lian commented at 1:17 AM on November 22, 2013: contributor

No description provided.

cleanup error message consistency

relicts from the CTransaction refactor.

848fe68acd

in src/main.cpp:None in 1331ee8886 outdated
```
2041 | @@ -2042,7 +2042,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
2042 |                               REJECT_INVALID, "time in future");
2043 |  
2044 |      // First transaction must be coinbase, the rest must not be
2045 | -    if (block.vtx.empty() || !block.vtx[0].IsCoinBase())
2046 | +    if (!block.vtx[0].IsCoinBase())
```
laanwj commented at 3:10 AM on November 22, 2013:

That's not what I would call a minor typo cleanup. Why are you removing this check?

lian commented at 3:47 AM on November 22, 2013:

please see the commit message for that change: https://github.com/lian/bitcoin/commit/1331ee88867459d34c953c1cb688b51c674b2d96

block.vtx.empty() is already checked 3 lines above in that same method.

Diapolo commented at 7:45 AM on November 22, 2013:

If this is the case I suggest to create a separate commit, this will speedup merging of uncontroversial changes and it catches more attention :).

lian commented at 6:22 PM on November 22, 2013:

its a seperate commit already, but i guess you mean pull request.

i don't see whats the issue with this change, its just git diffs fault for not showing more than 5 lines so everyone freaks out with just seeing the deleted part.

laanwj commented at 8:14 AM on November 23, 2013:

There is nothing wrong with the change itself @lian. But CheckBlock is one of the most critical functions in the client. So we're (understandably, I hope) a bit baffled to see a change to it in a pull request called "Minor typo cleanup". If you want to suggest changes to this function, please do it as a separate pull request with an appropriate name, so that the right people will look at it.

lian commented at 2:23 PM on November 23, 2013:

thanks laanwj, i moved the CheckBlock change into its own pull request now. #3311

in src/main.cpp:None in 1331ee8886 outdated

 551 | @@ -552,7 +552,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state)
 552 |                           REJECT_INVALID, "vout empty");
 553 |      // Size limits
 554 |      if (::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) > MAX_BLOCK_SIZE)
 555 | -        return state.DoS(100, error("CTransaction::CheckTransaction() : size limits failed"),
 556 | +        return state.DoS(100, error("CheckTransaction() : size limits failed"),

Diapolo commented at 7:44 AM on November 22, 2013:

While you are at this, AFAIK we should use Function : message, so remove the () as they don't add any information.

BitcoinPullTester commented at 3:02 PM on November 23, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/848fe68acdfbf9e046430e92e314159cecf8eb1e for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj referenced this in commit eef8a67abf on Nov 24, 2013
laanwj merged this on Nov 24, 2013
laanwj closed this on Nov 24, 2013
Bushstar referenced this in commit 015a2a2f57 on Apr 8, 2020
bitcoin locked this on Sep 8, 2021

Contributors

Linked (view graph)

#3311 minor cleanup in CheckBlock