rpc: Improve invalid vout value rpc error message #19956

pull n1rna wants to merge 1 commits into bitcoin:master from n1rna:rpc-invalid-vout-message changing 4 files +5 −5

n1rna commented at 9:47 PM on September 14, 2020: contributor

Since the vout value can start at 0, the error message for negative values can be improved to something like: vout cannot be negative.
n1rna force-pushed on Sep 14, 2020
fanquake added the label RPC/REST/ZMQ on Sep 14, 2020
theStack commented at 9:28 PM on September 15, 2020: member

Concept ACK -- "positive" means greater than zero by definition, so the error message was incorrect

I think you should squash those commits.

Welcome as new contributor by the way! :tada:
n1rna force-pushed on Sep 16, 2020
n1rna commented at 4:14 AM on September 16, 2020: contributor

Welcome as new contributor by the way!

THANKS @theStack
tryphe commented at 6:05 AM on September 16, 2020: contributor

ACK 89860a8b52c7c84b987169ce238d8e8bc809ae7b. Agree with the concept, changes are no-risk.
theStack approved
theStack commented at 9:28 AM on September 16, 2020: member

ACK https://github.com/bitcoin/bitcoin/commit/89860a8b52c7c84b987169ce238d8e8bc809ae7b 🆗
adaminsky commented at 1:25 AM on September 23, 2020: contributor

Concept ACK -- This error also appears some other places. Here are the first two I found from git grep "vout must be positive", but there were some others.

https://github.com/bitcoin/bitcoin/blob/d052f5e6b79b9600f929457e9280b441b772785b/src/bitcoin-tx.cpp#L599-L600 https://github.com/bitcoin/bitcoin/blob/ffaac6e6142a1cf61e8e12d7fc406fcb02300144/src/rpc/rawtransaction_util.cpp#L177-L179
n1rna force-pushed on Sep 24, 2020
n1rna commented at 4:49 PM on September 24, 2020: contributor

This error also appears some other places.

Addressed. @adaminsky
adaminsky commented at 5:45 PM on September 24, 2020: contributor

ACK 4be83808ba814dd5603dfcd72199ab723a4020f2.
MarcoFalke commented at 5:54 PM on September 24, 2020: member

Could be a scripted diff to show the replacement is complete?
n1rna commented at 5:59 PM on September 24, 2020: contributor

I was not familiar with scripted diff, I will replace my commit with with script diff enabled. @MarcoFalke
n1rna force-pushed on Sep 24, 2020
n1rna force-pushed on Sep 24, 2020

laanwj commented at 2:11 PM on September 30, 2020: member

Please insert an empty line in your commit message between the title of the commit and the body, otherwise it appears like this in one-line summary format:

* f56bff455f6b965cd4f50ec8e6fe2bf5981ca30d scripted diff: Improve invalid vout value rpc error message -BEGIN VERIFY SCRIPT- r() { sed -i 's/vout must be positive/vout cannot be negative/g' $1 } r src/bitcoin-tx.cpp r src/rpc/rawtransaction_util.cpp r src/wallet.cpp r test/functional/rpc_rawtransaction.py -END VERIFY SCRIPT- (Nima Yazdanmehr) (pull/19956/head)

The change itself looks good to me.

n1rna force-pushed on Sep 30, 2020
n1rna commented at 3:50 PM on September 30, 2020: contributor

Addressed @laanwj
MarcoFalke commented at 4:57 PM on September 30, 2020: member

Instead of hardcoding the file names, you can use $(git grep -l 'vout must be positive')
n1rna force-pushed on Sep 30, 2020
n1rna commented at 5:09 PM on September 30, 2020: contributor

Yes, much better now. Thanks for the suggestoin @MarcoFalke

scripted diff: Improve invalid vout value rpc error message

-BEGIN VERIFY SCRIPT-
r() { sed -i 's/vout must be positive/vout cannot be negative/g' $1 }
r $(git grep -l 'vout must be positive')
-END VERIFY SCRIPT-

f471a3be00

promag commented at 9:18 AM on October 1, 2020: member

Code review ACK f471a3be00c2b6433b8c258b716982c0539da13f.
fanquake approved
fanquake commented at 3:12 AM on October 3, 2020: member

ACK f471a3be00c2b6433b8c258b716982c0539da13f
fanquake merged this on Oct 3, 2020
fanquake closed this on Oct 3, 2020
Fabcien referenced this in commit 2d13fb531d on Nov 3, 2021
PastaPastaPasta referenced this in commit 6d29810863 on Dec 22, 2021
PastaPastaPasta referenced this in commit e672e0a314 on Dec 22, 2021
PastaPastaPasta referenced this in commit 7bfd262512 on Dec 22, 2021
PastaPastaPasta referenced this in commit 8d58fdf1a9 on Dec 28, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels