validation: Rename CheckInputs to CheckInputScripts #16658

pull jnewbery wants to merge 2 commits into bitcoin:master from jnewbery:2019-08-rename-CheckInputs changing 5 files +41 −39
  1. jnewbery commented at 4:13 PM on August 19, 2019: member

    CheckInputs() used to check no double spends, scripts & sigs and amounts. Since 832e074, the double spend and amount checks have been moved to CheckTxInputs(), and CheckInputs() now just validates input scripts. Rename the function to CheckInputScripts().

    Also fix incorrect comments.

  2. jnewbery commented at 4:13 PM on August 19, 2019: member

    This builds on #13868. Please review that PR first.

  3. DrahtBot added the label Tests on Aug 19, 2019
  4. DrahtBot added the label Validation on Aug 19, 2019
  5. DrahtBot commented at 6:29 PM on August 19, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17399 (validation: Templatize ValidationState instead of subclassing by jkczyz)

    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.

  6. jnewbery force-pushed on Aug 25, 2019
  7. jnewbery renamed this:
    [validation] Rename CheckInputs to CheckInputScripts
    validation: Rename CheckInputs to CheckInputScripts
    on Aug 25, 2019
  8. jnewbery renamed this:
    validation: Rename CheckInputs to CheckInputScripts
    [WIP] validation: Rename CheckInputs to CheckInputScripts
    on Aug 26, 2019
  9. michaelfolkson cross-referenced this on Sep 1, 2019 from issue doc: Update comment describing CheckInputs in validation.cpp by michaelfolkson
  10. fanquake commented at 2:39 AM on September 2, 2019: member

    Concept ACK

    This builds on #13868. Please review that PR first.

    I have reviewed the base PR.

    Should 046cd7555e909d9f62a14c44ef0c09eac209b287 be a scripted-diff? This should produce the same change:

    gsed -i 's/\<CheckInputs\>/CheckInputScripts/g' src/*.h src/*.cpp src/**/*.h src/**/*.cpp

    This needs a couple functional test changes for the expected messages: https://github.com/bitcoin/bitcoin/blob/7d6f63cc2c2b9c4f07a43619eef0b7314474fffd/test/functional/feature_dersig.py#L127 https://github.com/bitcoin/bitcoin/blob/7d6f63cc2c2b9c4f07a43619eef0b7314474fffd/test/functional/feature_cltv.py#L141

  11. DrahtBot added the label Needs rebase on Sep 2, 2019
  12. jnewbery force-pushed on Sep 2, 2019
  13. jnewbery renamed this:
    [WIP] validation: Rename CheckInputs to CheckInputScripts
    validation: Rename CheckInputs to CheckInputScripts
    on Sep 2, 2019
  14. jnewbery commented at 3:25 PM on September 2, 2019: member

    Rebased and removed WIP tag.

    Thanks for the review @fanquake

    This needs a couple functional test changes for the expected messages.

    Fixed. Thanks.

    Should 046cd75 be a scripted-diff?

    I think this is small enough to be a non-scripted commit, and I'd need to add more to your suggested one-liner to make the test changes atomic with the validation.cpp changes. I'm leaving as it is for now, but can change if other reviewers agree that scripted would be better.

  15. DrahtBot removed the label Needs rebase on Sep 2, 2019
  16. fanquake cross-referenced this on Sep 3, 2019 from issue Remove unused fScriptChecks parameter from CheckInputs by Empact
  17. ajtowns commented at 11:57 AM on September 3, 2019: contributor

    I'd need to add more to your suggested one-liner to make the test changes atomic with the validation.cpp changes

    FWIW, I believe a valid scripted-diff line for 3623a8f946c55c3fd897a057ac38acb85817f593 would be:

    sed -i -E -e 's/CheckInputs\b/CheckInputScripts/g' $(git grep -l CheckInputs | grep -v doc/)

  18. in src/validation.cpp:1287 in 41d152860f outdated 
    1280 | @@ -1284,8 +1281,10 @@ void InitScriptExecutionCache() {
1281 |  }
1282 |  
1283 |  /**
1284 | - * Check whether all inputs of this transaction are valid (no double spends, scripts & sigs, amounts)
1285 | - * This does not modify the UTXO set.
1286 | + * Check whether all of this transaction's input scripts succeed.
1287 | + *
1288 | + * This involves ECDSA signature checks so can be computationally intensive. This function should
1289 | + * only be called after the basic tx amount tests in CheckTxInputs passed.
    MarcoFalke commented at 8:05 PM on September 3, 2019:

    This seems to imply that when the basic tx amount tests pass, the amount fields in the tx are correct. However, you can only know that the amount fields are correct when all subsequent check (including signature checks have been run). I'd rather say something like "all cheap sanity checks passed", to be not overly specific.

    jnewbery commented at 10:32 PM on September 4, 2019:

    agree. Fixed

  19. jnewbery force-pushed on Sep 4, 2019
  20. jnewbery commented at 10:32 PM on September 4, 2019: member

    FWIW, I believe a valid scripted-diff line for 3623a8f would be...

    I've updated the commit to use your scripted diff one-liner. Thanks!

  21. michaelfolkson commented at 12:49 PM on September 7, 2019: contributor

    Concept ACK. I ran a git grep on CheckInputs in this PR branch and apart from old release notes that obviously won't change, there is a ValidateCheckInputsForAllFlags function in src/test/txvalidationcache_tests.cpp, a CheckInputsAndUpdateCoins function in src/txmempool.cpp and CheckInputsFromMempoolAndCache function in src/validation.cpp. Should they be changed too?

  22. fanquake commented at 8:10 AM on September 8, 2019: member

    @jnewbery Looks like the scripted diff needs fixing:

    $ set -o errexit; source ./ci/lint/06_script.sh
Error: script block marker but no scripted-diff in title
Failed
  23. jnewbery commented at 2:59 PM on September 8, 2019: member

    Looks like the scripted diff needs fixing

    Thanks. Will fix next week.

  24. jnewbery force-pushed on Sep 10, 2019
  25. jnewbery commented at 12:56 PM on September 10, 2019: member

    Looks like the scripted diff needs fixing

    should be fixed

  26. DrahtBot added the label Needs rebase on Sep 18, 2019
  27. jnewbery force-pushed on Oct 1, 2019
  28. jnewbery commented at 2:18 PM on October 1, 2019: member

    rebased

  29. DrahtBot removed the label Needs rebase on Oct 1, 2019
  30. MarcoFalke commented at 7:21 PM on October 11, 2019: member

    ACK 3ee9ad76803c0f9fdcb21ac1ad6162df79df07da thanks for fixing documentation

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 3ee9ad76803c0f9fdcb21ac1ad6162df79df07da thanks for fixing documentation
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUg1jQwAystQNbOYRs28u4UXVAow/cWruoKgEWgtVxmszf3FIl6XOkkK35LHsXD8
P0YZ/7XAaGIXufQYGkMr4Iaq/lONZjw6oQOqkgCNfq5UjPS4JesEuj/QtKwrQGPp
xFYdGzuU4CE5NQlAtx1bXK5CebOc03FgFYuN9u74579t7nl0P5dp/VjaIUVcdZFE
Wgxgwfa2DboOnI5sv9drQvcX9nLJcUPfBZWufb9RUCE+rklB0hjTomISPUbF75fd
2XFUuOFgIonwwrFkgg6ChTaecDHaphCsttauGRigKxWyOsk3NIjwc8DfAyDK2+H8
HZcMguMNy183bqRFbzbL/VEJD3rDYvj8dtg8cwGlejcP20tj3g1yZqJIerqXU8GZ
5t6xENcNWHjOuL8HpuRg9JiRBM1E12/c39IUAKaG/5W3K4tODI3ziRfu8JQMKpDH
CrDHoaB23mTzzjYxq7R9aMWDSdA0oLzTKL1kGi1Y3SEOWs06e7JSAMhdnoOiueg4
st2HgDxb
=CD9a
-----END PGP SIGNATURE-----

    Timestamp of file with hash 2fb4b49625388db44b56838d9617fb9d4d07331a02b3a6db6af05658e13fad54 -

    </details>

  31. MarcoFalke removed the label Tests on Oct 11, 2019
  32. MarcoFalke added the label Docs on Oct 11, 2019
  33. MarcoFalke cross-referenced this on Oct 17, 2019 from issue bench: Add BIP 143 sighash benchmark by MarcoFalke
  34. DrahtBot added the label Needs rebase on Oct 25, 2019
  35. jnewbery commented at 1:31 PM on October 25, 2019: member

    rebased

  36. jnewbery force-pushed on Oct 25, 2019
  37. fanquake removed the label Needs rebase on Oct 25, 2019
  38. MarcoFalke commented at 3:44 PM on October 25, 2019: member

    re-ACK 44d20b86b74445adb6d96f20e44efd1afae36035 only change is rebase the scripted diff

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

re-ACK 44d20b86b74445adb6d96f20e44efd1afae36035 only change is rebase the scripted diff
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhQBgv/f7OOsZwi+esjzjBssh0Bf6JMXRjjbZ+ARDo7KEqKjlf6x09B1ViMqC+X
+emq0IRC9dNzDLW6foblXUGvNwwsBCoULWmI7gXL+m6wZxkD2Byiuq1RDrLLvty1
cadiGJUbhIrMzUHXPsYPx/nt/Wq3kRc9UX09DU/JbkjBcZYYYsrMy96mqPt+2q+e
/hQ/1JsfofAqqS6cFNSNvnxSUG/vywJb6dgKd/c5+2SYAuZ1Nl4eTQiTeueP67nL
lypZt55WRlWoGo+1zUQIE6dDQxkfyNJfdsRQa8tNWS8wRdsBJNafuNRPiRpTcV6T
PJKq/J5syT7AYBQiyfyLyqYKaPve4j/nR4opIBtnZZzc02eQZH0MnHPhpamzV+LO
/Mqm5IYPC7xR5csWzTJas7lRaum81DWSKVowL7MO69M+TUmKH2K4n7vZ0kj1YKCY
dxvTZIzkglBR7uD9vRKUPv7KhVTNPiCSm1SreLyUSJJuE82Mex1JXAXt4Hj7UUSH
foyxWkSG
=oHWs
-----END PGP SIGNATURE-----

    Timestamp of file with hash 0630b39dafbd879abe6610f1a999b11bd28a1e6e9f1c97ebdfa437aa875e56fa -

    </details>

  39. promag commented at 3:55 PM on October 25, 2019: member

    TIL git grep -l CheckInputs -- ':!doc'.

    Code review ACK.

  40. DrahtBot added the label Needs rebase on Oct 30, 2019
  41. jnewbery force-pushed on Oct 30, 2019
  42. jnewbery commented at 4:47 PM on October 30, 2019: member

    Rebased on master

  43. DrahtBot removed the label Needs rebase on Oct 30, 2019
  44. DrahtBot commented at 4:31 PM on November 7, 2019: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->Needs rebase

  45. DrahtBot added the label Needs rebase on Nov 7, 2019
  46. MarcoFalke commented at 5:18 PM on November 7, 2019: member

    This is a trivial fix that should be ready for merge after rebase

  47. scripted-diff: [validation] Rename CheckInputs to CheckInputScripts
    CheckInputs() used to check no double spends, scripts & sigs and amounts. Since
832e0744cb8b1e1625cdb19b257f97316ac16a90, the double spend and amount checks
have been moved to CheckTxInputs(), and CheckInputs() now just validates
input scripts. Rename the function to CheckInputScripts().

-BEGIN VERIFY SCRIPT-
sed -i -E -e 's/CheckInputs\b/CheckInputScripts/g' $(git grep -l CheckInputs | grep -v doc/)
-END VERIFY SCRIPT-
    6f6465cefc
  48. [validation] fix comments in CheckInputScripts() 3bd8db80d8
  49. jnewbery force-pushed on Nov 7, 2019
  50. jnewbery commented at 6:53 PM on November 7, 2019: member

    Rebased

  51. fanquake removed the label Needs rebase on Nov 7, 2019
  52. MarcoFalke commented at 7:04 PM on November 7, 2019: member

    re-ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7, did the rebase myself, checked the scripted diff 👡

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

re-ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7, did the rebase myself, checked the scripted diff 👡
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUjh4wv/WSY4eMrYXocYlXm6kXbisKRQM+4z5HS9drtint6Mf3u/Zj3zioAxEApI
sIk6tuwzracrSYy2D6YtZnSSjpMwrtCRYy48OOt8e0WDnlUpZwTUyjCwp+l4nsMa
Bc+MUtTe9Eq8ICEAyFbNVGr+yvt18WlqM/h5DEIchFlGIE76BajPBTRxnf1QVe5o
OiC7ErXnpj79LzBYP5MF8/R7AMb66y+++Xop9pZeMkLAWM//0exTlsGrJU7V/GJW
2Mx/wApRUFjTz/NNDl5iArpayOa7KFcPZ9MhzvfBV0v+vihH3HzDKmvT9NYTDgox
6WvsBr1nj2F1ONV/fZQ9h8SK0jj4ewguDgNJ8cwB2uJQBqUOaWayNNNbE1egJIOv
33VpHuFdUIHsuRJMufZPpuNrK2/h0HguHVunuIM8jIvdG0dt+ioFYabXmKkywPi8
EQjw/j4DyvGzKILVKFpHatAFHLET87oGMH2Ph7ComQKOmYjsuyXvnys04y6KOwlg
4n7UJzLs
=3RbB
-----END PGP SIGNATURE-----

    Timestamp of file with hash 4aa6710c657c2553e2de927e5918efcd786d8c666d5ddcdb193f75684907f609 -

    </details>

  53. promag commented at 7:06 PM on November 7, 2019: member

    ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7 :trollface:

  54. MarcoFalke referenced this in commit 3f8dbcd655 on Jan 2, 2020
  55. MarcoFalke merged this on Jan 2, 2020
  56. MarcoFalke closed this on Jan 2, 2020
  57. jnewbery deleted the branch on Jan 2, 2020
  58. sidhujag referenced this in commit 607d4e89fd on Jan 2, 2020
  59. MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
  60. sidhujag referenced this in commit a49fcbce2c on Nov 10, 2020
  61. Fabcien referenced this in commit 925120cac2 on Jan 13, 2021
  62. bitcoin locked this on Feb 15, 2022
Contributors
jnewberyDrahtBotfanquakeajtownsMarcoFalkemichaelfolksonpromag
Labels
DocsValidation
Linked (view graph)
#13868 Remove unused fScriptChecks parameter from CheckInputs#16781 doc: Update comment describing CheckInputs in validation.cpp#17175 bench: Add BIP 143 sighash benchmark
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 07:54 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me