doc: Fix typos pointed out by lint-spelling #24203

brunoerg commented at 10:20 PM on January 29, 2022: member

Occuring -> occurring (random.h) Covert -> convert (chacha_poly_aead.cpp) Fix nWe false positive in blockchain.cpp (https://github.com/bitcoin/bitcoin/pull/24203#issuecomment-1025116962)

Got it by linter, other ones are false positives.

in src/random.h:92 in 0abddf33aa outdated

  88 | @@ -89,7 +89,7 @@ constexpr auto GetRandMillis = GetRandomDuration<std::chrono::milliseconds>;
  89 |   * is memoryless and should be used for repeated network events (e.g. sending a
  90 |   * certain type of message) to minimize leaking information to observers.
  91 |   *
  92 | - * The probability of an event occuring before time x is 1 - e^-(x/a) where a
  93 | + * The probability of an event occurring before time x is 1 - e^-(x/a) where a

jonatack commented at 10:36 PM on January 29, 2022:

Why only fix this one? The linter shows a few more. (Some are false positives and should be handled differently).

bitcoin (master)₿ test/lint//lint-spelling.sh 
src/crypto/chacha_poly_aead.cpp:76: covert ==> convert
src/random.h:92: occuring ==> occurring
src/rpc/blockchain.cpp:794: nWe ==> new
src/util/syscall_sandbox.cpp:128: creat ==> create
test/functional/data/rpc_decodescript.json:81: ba ==> by, be
test/functional/data/rpc_decodescript.json:84: ba ==> by, be
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt

brunoerg commented at 10:59 PM on January 29, 2022:

Most of that are false positives. Let me check.

brunoerg commented at 11:02 PM on January 29, 2022:

covert => convert is legit. Just checked it, will push here.

jonatack commented at 11:02 PM on January 29, 2022:

Yes. When a pull fixes the spelling linter results, it usually ensures the result is clean in one commit and doesn't leave false positives.

brunoerg commented at 11:07 PM on January 29, 2022:

Make sense, great. Just updated it, thanks!

PastaPastaPasta commented at 4:05 AM on January 30, 2022:

You didn't push fyi if you intended to

brunoerg commented at 11:39 AM on January 30, 2022:

I did it. Only covert and occuring are errors, other ones are false positives.

jonatack commented at 3:44 PM on January 30, 2022:

Why not fix the false positives so the linter output is clean?

DrahtBot added the label Docs on Jan 29, 2022

brunoerg renamed this:
~~doc: Fix typo in random.h~~
doc: Fix typo in random.h and chacha_poly_aead.cpp
on Jan 29, 2022

brunoerg force-pushed on Jan 29, 2022

DrahtBot commented at 5:00 AM on January 30, 2022: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#23813 (Add test and docs for getblockfrompeer with pruning by fjahr)
#23441 (fuzz: Differential fuzzing for ChaCha20Forward4064-Poly1305@bitcoin cipher suite by stratospher)
#23233 (BIP324: Add encrypted p2p transport {de}serializer by dhruv)
#20962 (Alter the ChaCha20Poly1305@Bitcoin AEAD to the new specification by jonasschnelli)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

prusnak approved

prusnak commented at 9:52 AM on January 30, 2022: contributor

utACK 9255261aba5663065abda29867078f0faa288cd7

shaavan commented at 10:48 AM on January 30, 2022: contributor

Concept ACK

I agree with @jonatack suggestion. This PR's scope should be broadened to correct all the lint spelling check instances, either by fixing the typo or marking the instances as false positive.

After the recent update of the PR. There are still three instances in the lint-spelling.sh test that needs to be addressed:

nWe
creat
ba

Screenshot from 2022-01-30 16-15-04

I would also suggest renaming the PR heading to better to infer the new general goal of this PR.

prusnak commented at 10:51 AM on January 30, 2022: contributor

nWe

creat

ba

These are all three false positives. Suggestion for fixing the nWe one:

--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -790,10 +790,10 @@ static RPCHelpMan getblockfrompeer()
 {
     return RPCHelpMan{
         "getblockfrompeer",
-        "Attempt to fetch block from a given peer.\n"
-        "\nWe must have the header for this block, e.g. using submitheader.\n"
-        "Subsequent calls for the same block and a new peer will cause the response from the previous peer to be ignored.\n"
-        "\nReturns an empty JSON object if the request was successfully scheduled.",
+        "Attempt to fetch block from a given peer.\n\n"
+        "We must have the header for this block, e.g. using submitheader.\n"
+        "Subsequent calls for the same block and a new peer will cause the response from the previous peer to be ignored.\n\n"
+        "Returns an empty JSON object if the request was successfully scheduled.",
         {
             {"block_hash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash to try to fetch"},
             {"peer_id", RPCArg::Type::NUM, RPCArg::Optional::NO, "The peer to fetch it from (see getpeerinfo for peer IDs)"},

brunoerg commented at 11:40 AM on January 30, 2022: member

Nice suggestion @prusnak for nWe. Going to address it.

brunoerg renamed this:
~~doc: Fix typo in random.h and chacha_poly_aead.cpp~~
doc: Fix typos pointed by lint-spelling
on Jan 30, 2022

brunoerg renamed this:
~~doc: Fix typos pointed by lint-spelling~~
doc: Fix typos pointed out by lint-spelling
on Jan 30, 2022

brunoerg force-pushed on Jan 30, 2022

in src/rpc/blockchain.cpp:794 in 80045cf864 outdated

 789 | @@ -790,8 +790,8 @@ static RPCHelpMan getblockfrompeer()
 790 |  {
 791 |      return RPCHelpMan{
 792 |          "getblockfrompeer",
 793 | -        "Attempt to fetch block from a given peer.\n"
 794 | -        "\nWe must have the header for this block, e.g. using submitheader.\n"
 795 | +        "Attempt to fetch block from a given peer.\n\n"
 796 | +        "We must have the header for this block, e.g. using submitheader.\n"

prusnak commented at 11:48 AM on January 30, 2022:

Can we fix the nReturns below just for consistency?

brunoerg commented at 11:58 AM on January 30, 2022:

Yes, agreed.

prusnak changes_requested

doc: Fix typos pointed out by lint-spelling bad0e7f521

brunoerg force-pushed on Jan 30, 2022

brunoerg commented at 12:00 PM on January 30, 2022: member

force-pushed addressing @prusnak's comments

w0xlt approved

w0xlt commented at 3:36 PM on January 30, 2022: contributor

ACK bad0e7f

jonatack commented at 3:42 PM on January 30, 2022: member

test/lint/lint-spelling.sh still returns:

src/util/syscall_sandbox.cpp:128: creat ==> create
test/functional/data/rpc_decodescript.json:81: ba ==> by, be
test/functional/data/rpc_decodescript.json:84: ba ==> by, be

Suggestion

--- a/test/lint/lint-spelling.ignore-words.txt
+++ b/test/lint/lint-spelling.ignore-words.txt
@@ -1,6 +1,9 @@
 asend
+ba
 blockin
 cachable
+creat
 fo
 fpr
 hights

(creat is a valid linux syscall)

katesalazar commented at 3:54 PM on January 30, 2022: contributor

Concept ACK

lint: add creat and ba into ignore-words for lint-spelling 58ccc88541

brunoerg commented at 7:17 PM on January 30, 2022: member

force-pushed addressing @jonatack's comments. Thank you for the reviews!

prusnak approved

prusnak commented at 10:31 PM on January 30, 2022: contributor

ACK 58ccc885413d66b4fa581574360febfa27eb545f

tested that ./test/lint/lint-spelling.sh returns no errors

PastaPastaPasta approved

PastaPastaPasta commented at 7:15 AM on January 31, 2022: contributor

utACK, it's good to fix this. The changes make sense, I did not validate that the linter output is clean now.

MarcoFalke merged this on Jan 31, 2022

MarcoFalke closed this on Jan 31, 2022

sidhujag referenced this in commit 6d1cbe30f7 on Feb 1, 2022

DrahtBot locked this on Jan 31, 2023