Add absurdly high fee message to validation state

shaulkf commented at 2:08 am on March 17, 2015: contributor

Currently error message isn’t propagating to RPC. Not sure how reject constants should be numbered, for now I arbitrarily chose 0x44, please advise.

sipa commented at 9:53 am on March 17, 2015: member

There is no need for a reject code, as this can never trigger for peer-to-peer initiated transactions, so it can never result in a reject message.

If you really need a reject code, add a REJECT_LOCAL or something, reserved for conditions that can only trigger locally (and mark it is an implementation detail, not related to BIP 61.

shaulkf commented at 1:02 pm on March 17, 2015: contributor

@sipa I believe there should be a code, in case RPC users want to catch the error without having to match the text. Any convention for numbering or could I choose any arbitrary code? (0x60 perhaps)

sipa commented at 1:14 pm on March 17, 2015: member

Well you can’t pick anything that could potentially be later used in the P2P protocol, which why I would prefer no actual code. If there really is a need for RPC error codes separately from what the peer to peer protocol does, perhaps there should just be two codes. In general, I think it’s not possible to predict all possible reason for rejecting things, and not really viable to have codes for everything.

shaulkf commented at 1:42 pm on March 17, 2015: contributor

I’m not sure what you mean by adding REJECT_LOCAL, should I create an error code which will be overloaded for any local error? How about REJECT_LOCAL = 0x00.

shaulkf force-pushed on Mar 17, 2015

laanwj commented at 8:11 am on March 18, 2015: member

You could use reject codes > 0x100 for local-only conditions. These cannot be passed over the P2P network.

laanwj added the label RPC on Mar 18, 2015

shaulkf commented at 6:49 pm on March 18, 2015: contributor

@laanwj Is it okay to change reject message codes to unsigned int for this purpose? If not, please advise is we can assign a dedicated 2 byte code (e.g. 0x00 or 0xFF) for all REJECT_LOCAL errors. I prefer the first solution as RPC users can catch errors with explicit error codes.

laanwj commented at 4:41 pm on March 24, 2015: member

@shaulkf Changing the internal type is fine with me, otherwise I wouldn’t have suggested it. But be careful that you don’t accidentally change the type that is sent in the protocol.

laanwj commented at 7:07 am on April 8, 2015: member

utACK

luke-jr commented at 5:00 am on June 24, 2015: member

Needs rebase. I suggest leaving the const reject message codes (now in consensus/validation.h) as unsigned char, changing CValidationState as you already do, and putting the local codes in main.h (for now).

luke-jr commented at 5:27 am on June 24, 2015: member

Also:

0main.cpp: In function ‘void InvalidBlockFound(CBlockIndex*, const CValidationState&)’:
1main.cpp:1369:56: warning: narrowing conversion of ‘(& state)->CValidationState::GetRejectCode()’ from ‘unsigned int’ to ‘unsigned char’ inside { } is ill-formed in C++11 [-Wnarrowing]
2             CBlockReject reject = {state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), pindex->GetBlockHash()};
3                                                        ^

paveljanik commented at 9:59 pm on June 26, 2015: contributor

Needs rebase. I’d prefer to have something in really soon. RPC returns no clean error message and it is boring to look up the actual error in the debug log.

shaulkf force-pushed on Jun 26, 2015

shaulkf commented at 10:51 pm on June 26, 2015: contributor

Rebased and changed according to @luke-jr’s recommendation

shaulkf force-pushed on Jun 30, 2015

shaulkf commented at 5:20 pm on June 30, 2015: contributor

Thanks @paveljanik, pushed a fix and rebased, this is a remnant from a change I ended up reverting. I agree regarding Invalid and DoS but this should be in a separate PR as it requires reordering the arguments.

Add absurdly high fee message to validation state (for RPC propagation) a651403e09

shaulkf force-pushed on Jun 30, 2015

luke-jr commented at 6:12 am on July 2, 2015: member

@shaulkf Please fix the warning.

paveljanik commented at 6:33 am on July 2, 2015: contributor

Something like this needed?

 0diff --git a/src/main.cpp b/src/main.cpp
 1index f67f1fd..5229f6f 100644
 2--- a/src/main.cpp
 3+++ b/src/main.cpp
 4@@ -182,7 +182,7 @@ namespace {
 5 namespace {
 6
 7 struct CBlockReject {
 8-    unsigned char chRejectCode;
 9+    unsigned int chRejectCode;
10     string strRejectReason;
11     uint256 hashBlock;
12 };

luke-jr commented at 6:36 am on July 2, 2015: member

Not sure, that might change the protocol. Maybe:

0CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), pindex->GetBlockHash()};

paveljanik commented at 6:42 am on July 2, 2015: contributor

Right.

laanwj commented at 4:40 pm on August 5, 2015: member

Need this for #6519

sipa commented at 4:51 pm on August 5, 2015: member

utACK.

Maybe this also needs an assert before sending out a reject message that the chRejectCode is in the right range.

laanwj commented at 4:53 pm on August 5, 2015: member

Yes, good point. I’ll add it during merge.

laanwj merged this on Aug 5, 2015

laanwj closed this on Aug 5, 2015

laanwj referenced this in commit a0625b8085 on Aug 5, 2015

in src/main.h: in a651403e09

496@@ -497,4 +497,7 @@ extern CBlockTreeDB *pblocktree;
497  */
498 int GetSpendHeight(const CCoinsViewCache& inputs);
499 
500+/** local "reject" message codes for RPC which can not be triggered by p2p trasactions */

Diapolo commented at 6:45 am on August 6, 2015:

Nit: Spelling error trasactions. See https://github.com/theuni/bitcoin/pull/48 @theuni Will there be a Trivial pull before 0.11?

laanwj referenced this in commit 9fc6ed6003 on Dec 7, 2015

laanwj referenced this in commit 44fef99e66 on Dec 10, 2015

furszy referenced this in commit 8d7e7808d0 on Jun 20, 2020

zkbot referenced this in commit 19a8c45f42 on Aug 13, 2021

zkbot referenced this in commit 8e9f44cbe2 on Aug 13, 2021

zkbot referenced this in commit cf9a0799b4 on Aug 17, 2021

MarcoFalke locked this on Sep 8, 2021

Add absurdly high fee message to validation state #5913