test: passing a negative value to `-peertimeout` should throw an error #25505

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2022-06-peertimeout-negative changing 1 files +5 −0

brunoerg commented at 8:29 PM on June 29, 2022: member

This PR adds test coverage for the following init error: https://github.com/bitcoin/bitcoin/blob/5bc10b39abbcb77638161902ccd1225139bc7cc0/src/init.cpp#L967-L970
test: passing a negative value to `-peertimeout` should throw an error 694669e145
kristapsk approved
kristapsk commented at 8:47 PM on June 29, 2022: contributor

utACK 694669e1458ec16a88e6be7c6723144e778fa879
kristapsk commented at 8:48 PM on June 29, 2022: contributor

While looking at this, noticed that it gives the same error message about "cannot be negative" if you try to start with non-integer -peertimeout, which seems wrong.
ghost commented at 9:17 PM on June 29, 2022: none
While looking at this, noticed that it gives the same error message about "cannot be negative" if you try to start with non-integer -peertimeout, which seems wrong.
```
- return InitError(Untranslated("peertimeout cannot be configured with a negative value."));
+ return InitError(Untranslated("Invalid value for peertimeout: %d", peer_connect_timeout));
```
Maybe this could fix it
brunoerg commented at 9:24 PM on June 29, 2022: member

While looking at this, noticed that it gives the same error message about "cannot be negative" if you try to start with non-integer -peertimeout, which seems wrong.

Yea, just noticed it. Same if you pass 0

brunoerg commented at 9:26 PM on June 29, 2022: member

- return InitError(Untranslated("peertimeout cannot be configured with a negative value."));
+ return InitError(Untranslated("Invalid value for peertimeout: %d. It should be a positive integer., peer_connect_timeout));

@1440000bytes maybe this way? I can address this change in this PR if other reviewers agree.

DrahtBot added the label Tests on Jun 29, 2022
ghost commented at 10:11 PM on June 29, 2022: none

@1440000bytes maybe this way? I can address this change in this PR if other reviewers agree.

I added another error message in https://github.com/bitcoin/bitcoin/pull/25506
unknown approved
unknown commented at 10:13 PM on June 29, 2022: none

ACK https://github.com/bitcoin/bitcoin/pull/25505/commits/694669e1458ec16a88e6be7c6723144e778fa879
kristapsk commented at 10:25 PM on June 29, 2022: contributor

Probably it's good idea to first decide about merging or not merging #25506 and then possibly add additional test case here.
MarcoFalke closed this on Jun 30, 2022
brunoerg commented at 1:12 PM on June 30, 2022: member

Why is this closed? @MarcoFalke I intended to rebase it
kristapsk commented at 1:23 PM on June 30, 2022: contributor

It was auto-closed by github-merge.sh script, because PR description contained "Fixes" with link to this PR, script didn't understand that it was link to a comment.
brunoerg commented at 1:25 PM on June 30, 2022: member

@kristapsk oh sorry, didn't notice it. Anyway, I opened #25511 in order to this one. thanks!
MarcoFalke referenced this in commit b6cf0f8848 on Jun 30, 2022
sidhujag referenced this in commit 97f981e659 on Jun 30, 2022
sidhujag referenced this in commit 8d61dab2e7 on Jun 30, 2022
DrahtBot locked this on Jun 30, 2023

Contributors

Labels

Linked (view graph)

#25506 Rephrase error message for invalid value of `-peertimeout`#25511 test: non-positive integer value to `-peertimeout` should throw an error