Skip to content

Conversation

achow101
Copy link
Member

It was reported on IRC that scantxoutset's API was broken in 0.19.0:

<belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
<belcher> im on regtest, in case its important
<harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
<harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
<jonatack> Same for me as well
<harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.

As noted in the conversation, previously, the second argument of scanobjects is only required for the start action. Stop and abort actions did not and could work without them.

It appears that this was broken by #16240 which enforced the size of the arguments to match the listed required arguments.

To fix this issue, this PR makes the scanobjects argument an optional argument. Then only in the start action do we check whether the scanobjects argument is there and throw an informative error about that. Also a test is added for this case.

The second argument of scanobjects is only required for the start action.
Stop and abort actions do not need this.
@promag
Copy link
Contributor

promag commented Dec 12, 2019

Approach ACK.

@maflcko
Copy link
Member

maflcko commented Dec 12, 2019

ACK, this was introduced by me in fa0ad4e

@maflcko
Copy link
Member

maflcko commented Dec 12, 2019

Probably because the previous documentation incorrectly labeled this as "required". See https://bitcoincore.org/en/doc/0.17.0/rpc/blockchain/scantxoutset/

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 7d26357.

@laanwj
Copy link
Member

laanwj commented Dec 15, 2019

ACK 7d26357
Thank you for fixing a clearly defined bug with a clear, minimal change.

laanwj added a commit that referenced this pull request Dec 15, 2019
…t action

7d26357 rpc: require second argument only for scantxoutset start action (Andrew Chow)

Pull request description:

  It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:

  ```
  <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
  <belcher> im on regtest, in case its important
  <harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
  <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
  <jonatack> Same for me as well
  <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
  ```

  As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.

  It appears that this was broken by #16240 which enforced the size of the arguments to match the listed required arguments.

  To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.

ACKs for top commit:
  laanwj:
    ACK 7d26357
  promag:
    ACK 7d26357.

Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
@laanwj laanwj merged commit 7d26357 into bitcoin:master Dec 15, 2019
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 16, 2019
…et start action

7d26357 rpc: require second argument only for scantxoutset start action (Andrew Chow)

Pull request description:

  It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:

  ```
  <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
  <belcher> im on regtest, in case its important
  <harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
  <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
  <jonatack> Same for me as well
  <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
  ```

  As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.

  It appears that this was broken by bitcoin#16240 which enforced the size of the arguments to match the listed required arguments.

  To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.

ACKs for top commit:
  laanwj:
    ACK 7d26357
  promag:
    ACK 7d26357.

Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 3, 2020
The second argument of scanobjects is only required for the start action.
Stop and abort actions do not need this.

Github-Pull: bitcoin#17728
Rebased-From: 7d26357
@fanquake fanquake mentioned this pull request Jan 3, 2020
luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Jan 5, 2020
The second argument of scanobjects is only required for the start action.
Stop and abort actions do not need this.

Github-Pull: bitcoin#17728
Rebased-From: 7d26357
@fanquake
Copy link
Member

fanquake commented Jan 6, 2020

Being backported in 17858.

laanwj added a commit that referenced this pull request Jan 8, 2020
99b5407 scripts: fix check-symbols & check-security argument passing (fanquake)
4330a1e Update msvc build for Visual Studio 2019 v16.4 (Aaron Clauson)
b0f9b8e Moves vcpkg list to a text file and updates the appveyor job and readme to use it. (Aaron Clauson)
cd7b3b2 Updated appveyor config:  - Update build image from Visual Studio 2017 to Visual Studio 2019.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added commands to update vcpkg port files (this does not update already installed packages).  - Updated vcpkg package list as per #17309.  - Removed commands setting common project file options. Now done via common.init.vcxproj include.  - Changed msbuild verbosity from normal to quiet. Normal rights a LOT of logs and impacts appveyor job duration. Updated msvc project configs:  - Updated platform toolset from v141 to v142.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added ignore for linker warning building bitcoin-qt program.  - Added missing util/str.cpp class file to test_bitcoin project file. (Aaron Clauson)
112144d Add missing typeinfo includes (Wladimir J. van der Laan)
1a6a534 net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan)
c0dc728 test: fix bitcoind already running warnings on macOS (fanquake)
5276b0e util: Add missing headers to util/fees.cpp (Hennadii Stepanov)
4d7875c rpc: require second argument only for scantxoutset start action (Andrew Chow)
bda2f5b cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice (Harris)
d14ab7c gui: disable File->CreateWallet during startup (fanquake)
b9f1bc0 wallet: unbreak with boost 1.72 (Jan Beich)

Pull request description:

  Backports the following PRs to the 0.19 branch:
  * #17654 - Unbreak build with Boost 1.72.0
  * #17695 - gui: disable File->CreateWallet during startup
  * #17687 - cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice
  * #17728 - rpc: require second argument only for scantxoutset start action
  * #17450 - util: Add missing headers to util/fees.cpp
  * #17488 - test: fix "bitcoind already running" warnings on macOS
  * #17762 - Log to net category for exceptions in ProcessMessages
  * #17364 - Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes
  * #17416 - Appveyor improvement - text file for vcpkg package list
  * #17736 - Update msvc build for Visual Studio 2019 v16.4
  * #17857 - scripts: fix symbol-check & security-check argument passing

  Fixes #17856.

ACKs for top commit:
  sipsorcery:
    ACK (tested: Windows 10 & msvc build) 99b5407.

Tree-SHA512: 91313de56fb0825e70a4be30ba0bf561b8c26d7dcf60549185df4f5e3524099398c828bb46faae807b631634d1afd5a1d397fb41e61ecfa0d746e4bf10b923cb
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Mar 13, 2020
[0.19] Backports bitcoin#17858
Unbreak build with Boost 1.72.0 bitcoin#17654
cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice bitcoin#17687
rpc: require second argument only for scantxoutset start action bitcoin#17728
wallet: Fix origfee return for bumpfee with feerate arg bitcoin#17643
test: fix "bitcoind already running" warnings on macOS bitcoin#17488
net: Log to net category for exceptions in ProcessMessages bitcoin#17762
Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes bitcoin#17364
Appveyor improvement - text file for vcpkg package list bitcoin#17416
Update msvc build for Visual Studio 2019 v16.4 bitcoin#17736
scripts: fix symbol-check & security-check argument passing bitcoin#17857
qt: Periodic translations update for 0.19 branch
IsUsedDestination should count any known single-key address bitcoin#17621
init: Stop indexes on shutdown after ChainStateFlushed callback. bitcoin#17897
qt: Translations update pre-rc1
wallet: Reset reused transactions cache bitcoin#17843
Squashed 'src/univalue/' changes from 7890db9..98261b1
0.19: Update univalue subtree bitcoin#18100
qt: Pre-rc2 translations update
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Mar 13, 2020
[0.19] Backports bitcoin#17858
Unbreak build with Boost 1.72.0 bitcoin#17654
cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice bitcoin#17687
rpc: require second argument only for scantxoutset start action bitcoin#17728
wallet: Fix origfee return for bumpfee with feerate arg bitcoin#17643
test: fix "bitcoind already running" warnings on macOS bitcoin#17488
net: Log to net category for exceptions in ProcessMessages bitcoin#17762
Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes bitcoin#17364
Appveyor improvement - text file for vcpkg package list bitcoin#17416
Update msvc build for Visual Studio 2019 v16.4 bitcoin#17736
scripts: fix symbol-check & security-check argument passing bitcoin#17857
qt: Periodic translations update for 0.19 branch
IsUsedDestination should count any known single-key address bitcoin#17621
init: Stop indexes on shutdown after ChainStateFlushed callback. bitcoin#17897
qt: Translations update pre-rc1
wallet: Reset reused transactions cache bitcoin#17843
Squashed 'src/univalue/' changes from 7890db9..98261b1
0.19: Update univalue subtree bitcoin#18100
qt: Pre-rc2 translations update
[0.19] Further 0.19 backports bitcoin#18218
build: don't embed a build-id when building libdmg-hfsplus bitcoin#18004
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 9, 2020
Summary:
> The second argument of scanobjects is only required for the start action. Stop and abort actions did not and could work without them.
>
> It appears that this was broken by #16240 which enforced the size of the arguments to match the listed required arguments.
>
> To fix this issue, this PR makes the scanobjects argument an optional argument. Then only in the start action do we check whether the scanobjects argument is there and throw an informative error about that. Also a test is added for this case.

This is a backport of Core [[bitcoin/bitcoin#17728 | PR17728]]

Test Plan:
`ninja && ./test/functional/test_runner.py rpc_scantxoutset`

In one terminal window, run:
`bitcoin-cli scantxoutset start  "[\"addr(1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2)\"]"`

In another terminal, check that the two following commands work:
```
bitcoin-cli scantxoutset status
bitcoin-cli scantxoutset abort
```

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8324
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
…et start action

7d26357 rpc: require second argument only for scantxoutset start action (Andrew Chow)

Pull request description:

  It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:

  ```
  <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
  <belcher> im on regtest, in case its important
  <harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
  <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
  <jonatack> Same for me as well
  <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
  ```

  As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.

  It appears that this was broken by bitcoin#16240 which enforced the size of the arguments to match the listed required arguments.

  To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.

ACKs for top commit:
  laanwj:
    ACK 7d26357
  promag:
    ACK 7d26357.

Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 9, 2021
99b5407 scripts: fix check-symbols & check-security argument passing (fanquake)
4330a1e Update msvc build for Visual Studio 2019 v16.4 (Aaron Clauson)
b0f9b8e Moves vcpkg list to a text file and updates the appveyor job and readme to use it. (Aaron Clauson)
cd7b3b2 Updated appveyor config:  - Update build image from Visual Studio 2017 to Visual Studio 2019.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added commands to update vcpkg port files (this does not update already installed packages).  - Updated vcpkg package list as per bitcoin#17309.  - Removed commands setting common project file options. Now done via common.init.vcxproj include.  - Changed msbuild verbosity from normal to quiet. Normal rights a LOT of logs and impacts appveyor job duration. Updated msvc project configs:  - Updated platform toolset from v141 to v142.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added ignore for linker warning building bitcoin-qt program.  - Added missing util/str.cpp class file to test_bitcoin project file. (Aaron Clauson)
112144d Add missing typeinfo includes (Wladimir J. van der Laan)
1a6a534 net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan)
c0dc728 test: fix bitcoind already running warnings on macOS (fanquake)
5276b0e util: Add missing headers to util/fees.cpp (Hennadii Stepanov)
4d7875c rpc: require second argument only for scantxoutset start action (Andrew Chow)
bda2f5b cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice (Harris)
d14ab7c gui: disable File->CreateWallet during startup (fanquake)
b9f1bc0 wallet: unbreak with boost 1.72 (Jan Beich)

Pull request description:

  Backports the following PRs to the 0.19 branch:
  * bitcoin#17654 - Unbreak build with Boost 1.72.0
  * bitcoin#17695 - gui: disable File->CreateWallet during startup
  * bitcoin#17687 - cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice
  * bitcoin#17728 - rpc: require second argument only for scantxoutset start action
  * bitcoin#17450 - util: Add missing headers to util/fees.cpp
  * bitcoin#17488 - test: fix "bitcoind already running" warnings on macOS
  * bitcoin#17762 - Log to net category for exceptions in ProcessMessages
  * bitcoin#17364 - Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes
  * bitcoin#17416 - Appveyor improvement - text file for vcpkg package list
  * bitcoin#17736 - Update msvc build for Visual Studio 2019 v16.4
  * bitcoin#17857 - scripts: fix symbol-check & security-check argument passing

  Fixes bitcoin#17856.

ACKs for top commit:
  sipsorcery:
    ACK (tested: Windows 10 & msvc build) 99b5407.

Tree-SHA512: 91313de56fb0825e70a4be30ba0bf561b8c26d7dcf60549185df4f5e3524099398c828bb46faae807b631634d1afd5a1d397fb41e61ecfa0d746e4bf10b923cb
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 9, 2021
99b5407 scripts: fix check-symbols & check-security argument passing (fanquake)
4330a1e Update msvc build for Visual Studio 2019 v16.4 (Aaron Clauson)
b0f9b8e Moves vcpkg list to a text file and updates the appveyor job and readme to use it. (Aaron Clauson)
cd7b3b2 Updated appveyor config:  - Update build image from Visual Studio 2017 to Visual Studio 2019.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added commands to update vcpkg port files (this does not update already installed packages).  - Updated vcpkg package list as per bitcoin#17309.  - Removed commands setting common project file options. Now done via common.init.vcxproj include.  - Changed msbuild verbosity from normal to quiet. Normal rights a LOT of logs and impacts appveyor job duration. Updated msvc project configs:  - Updated platform toolset from v141 to v142.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added ignore for linker warning building bitcoin-qt program.  - Added missing util/str.cpp class file to test_bitcoin project file. (Aaron Clauson)
112144d Add missing typeinfo includes (Wladimir J. van der Laan)
1a6a534 net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan)
c0dc728 test: fix bitcoind already running warnings on macOS (fanquake)
5276b0e util: Add missing headers to util/fees.cpp (Hennadii Stepanov)
4d7875c rpc: require second argument only for scantxoutset start action (Andrew Chow)
bda2f5b cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice (Harris)
d14ab7c gui: disable File->CreateWallet during startup (fanquake)
b9f1bc0 wallet: unbreak with boost 1.72 (Jan Beich)

Pull request description:

  Backports the following PRs to the 0.19 branch:
  * bitcoin#17654 - Unbreak build with Boost 1.72.0
  * bitcoin#17695 - gui: disable File->CreateWallet during startup
  * bitcoin#17687 - cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice
  * bitcoin#17728 - rpc: require second argument only for scantxoutset start action
  * bitcoin#17450 - util: Add missing headers to util/fees.cpp
  * bitcoin#17488 - test: fix "bitcoind already running" warnings on macOS
  * bitcoin#17762 - Log to net category for exceptions in ProcessMessages
  * bitcoin#17364 - Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes
  * bitcoin#17416 - Appveyor improvement - text file for vcpkg package list
  * bitcoin#17736 - Update msvc build for Visual Studio 2019 v16.4
  * bitcoin#17857 - scripts: fix symbol-check & security-check argument passing

  Fixes bitcoin#17856.

ACKs for top commit:
  sipsorcery:
    ACK (tested: Windows 10 & msvc build) 99b5407.

Tree-SHA512: 91313de56fb0825e70a4be30ba0bf561b8c26d7dcf60549185df4f5e3524099398c828bb46faae807b631634d1afd5a1d397fb41e61ecfa0d746e4bf10b923cb
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 9, 2021
99b5407 scripts: fix check-symbols & check-security argument passing (fanquake)
4330a1e Update msvc build for Visual Studio 2019 v16.4 (Aaron Clauson)
b0f9b8e Moves vcpkg list to a text file and updates the appveyor job and readme to use it. (Aaron Clauson)
cd7b3b2 Updated appveyor config:  - Update build image from Visual Studio 2017 to Visual Studio 2019.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added commands to update vcpkg port files (this does not update already installed packages).  - Updated vcpkg package list as per bitcoin#17309.  - Removed commands setting common project file options. Now done via common.init.vcxproj include.  - Changed msbuild verbosity from normal to quiet. Normal rights a LOT of logs and impacts appveyor job duration. Updated msvc project configs:  - Updated platform toolset from v141 to v142.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added ignore for linker warning building bitcoin-qt program.  - Added missing util/str.cpp class file to test_bitcoin project file. (Aaron Clauson)
112144d Add missing typeinfo includes (Wladimir J. van der Laan)
1a6a534 net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan)
c0dc728 test: fix bitcoind already running warnings on macOS (fanquake)
5276b0e util: Add missing headers to util/fees.cpp (Hennadii Stepanov)
4d7875c rpc: require second argument only for scantxoutset start action (Andrew Chow)
bda2f5b cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice (Harris)
d14ab7c gui: disable File->CreateWallet during startup (fanquake)
b9f1bc0 wallet: unbreak with boost 1.72 (Jan Beich)

Pull request description:

  Backports the following PRs to the 0.19 branch:
  * bitcoin#17654 - Unbreak build with Boost 1.72.0
  * bitcoin#17695 - gui: disable File->CreateWallet during startup
  * bitcoin#17687 - cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice
  * bitcoin#17728 - rpc: require second argument only for scantxoutset start action
  * bitcoin#17450 - util: Add missing headers to util/fees.cpp
  * bitcoin#17488 - test: fix "bitcoind already running" warnings on macOS
  * bitcoin#17762 - Log to net category for exceptions in ProcessMessages
  * bitcoin#17364 - Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes
  * bitcoin#17416 - Appveyor improvement - text file for vcpkg package list
  * bitcoin#17736 - Update msvc build for Visual Studio 2019 v16.4
  * bitcoin#17857 - scripts: fix symbol-check & security-check argument passing

  Fixes bitcoin#17856.

ACKs for top commit:
  sipsorcery:
    ACK (tested: Windows 10 & msvc build) 99b5407.

Tree-SHA512: 91313de56fb0825e70a4be30ba0bf561b8c26d7dcf60549185df4f5e3524099398c828bb46faae807b631634d1afd5a1d397fb41e61ecfa0d746e4bf10b923cb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 22, 2021
…et start action

7d26357 rpc: require second argument only for scantxoutset start action (Andrew Chow)

Pull request description:

  It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:

  ```
  <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
  <belcher> im on regtest, in case its important
  <harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
  <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
  <jonatack> Same for me as well
  <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
  ```

  As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.

  It appears that this was broken by bitcoin#16240 which enforced the size of the arguments to match the listed required arguments.

  To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.

ACKs for top commit:
  laanwj:
    ACK 7d26357
  promag:
    ACK 7d26357.

Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 22, 2021
…et start action

7d26357 rpc: require second argument only for scantxoutset start action (Andrew Chow)

Pull request description:

  It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:

  ```
  <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
  <belcher> im on regtest, in case its important
  <harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
  <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
  <jonatack> Same for me as well
  <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
  ```

  As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.

  It appears that this was broken by bitcoin#16240 which enforced the size of the arguments to match the listed required arguments.

  To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.

ACKs for top commit:
  laanwj:
    ACK 7d26357
  promag:
    ACK 7d26357.

Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 22, 2021
…et start action

7d26357 rpc: require second argument only for scantxoutset start action (Andrew Chow)

Pull request description:

  It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:

  ```
  <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
  <belcher> im on regtest, in case its important
  <harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
  <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
  <jonatack> Same for me as well
  <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
  ```

  As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.

  It appears that this was broken by bitcoin#16240 which enforced the size of the arguments to match the listed required arguments.

  To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.

ACKs for top commit:
  laanwj:
    ACK 7d26357
  promag:
    ACK 7d26357.

Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 23, 2021
99b5407 scripts: fix check-symbols & check-security argument passing (fanquake)
4330a1e Update msvc build for Visual Studio 2019 v16.4 (Aaron Clauson)
b0f9b8e Moves vcpkg list to a text file and updates the appveyor job and readme to use it. (Aaron Clauson)
cd7b3b2 Updated appveyor config:  - Update build image from Visual Studio 2017 to Visual Studio 2019.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added commands to update vcpkg port files (this does not update already installed packages).  - Updated vcpkg package list as per bitcoin#17309.  - Removed commands setting common project file options. Now done via common.init.vcxproj include.  - Changed msbuild verbosity from normal to quiet. Normal rights a LOT of logs and impacts appveyor job duration. Updated msvc project configs:  - Updated platform toolset from v141 to v142.  - Updated Qt static library from Qt5.9.7 to Qt5.9.8.  - Added ignore for linker warning building bitcoin-qt program.  - Added missing util/str.cpp class file to test_bitcoin project file. (Aaron Clauson)
112144d Add missing typeinfo includes (Wladimir J. van der Laan)
1a6a534 net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan)
c0dc728 test: fix bitcoind already running warnings on macOS (fanquake)
5276b0e util: Add missing headers to util/fees.cpp (Hennadii Stepanov)
4d7875c rpc: require second argument only for scantxoutset start action (Andrew Chow)
bda2f5b cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice (Harris)
d14ab7c gui: disable File->CreateWallet during startup (fanquake)
b9f1bc0 wallet: unbreak with boost 1.72 (Jan Beich)

Pull request description:

  Backports the following PRs to the 0.19 branch:
  * bitcoin#17654 - Unbreak build with Boost 1.72.0
  * bitcoin#17695 - gui: disable File->CreateWallet during startup
  * bitcoin#17687 - cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice
  * bitcoin#17728 - rpc: require second argument only for scantxoutset start action
  * bitcoin#17450 - util: Add missing headers to util/fees.cpp
  * bitcoin#17488 - test: fix "bitcoind already running" warnings on macOS
  * bitcoin#17762 - Log to net category for exceptions in ProcessMessages
  * bitcoin#17364 - Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes
  * bitcoin#17416 - Appveyor improvement - text file for vcpkg package list
  * bitcoin#17736 - Update msvc build for Visual Studio 2019 v16.4
  * bitcoin#17857 - scripts: fix symbol-check & security-check argument passing

  Fixes bitcoin#17856.

ACKs for top commit:
  sipsorcery:
    ACK (tested: Windows 10 & msvc build) 99b5407.

Tree-SHA512: 91313de56fb0825e70a4be30ba0bf561b8c26d7dcf60549185df4f5e3524099398c828bb46faae807b631634d1afd5a1d397fb41e61ecfa0d746e4bf10b923cb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 28, 2021
…et start action

7d26357 rpc: require second argument only for scantxoutset start action (Andrew Chow)

Pull request description:

  It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:

  ```
  <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
  <belcher> im on regtest, in case its important
  <harding> I can confirm `scantxoutset abort` returns the help doc on latest master.  Waiting for 0.18.1 to start now to attempt to reproduce there.
  <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
  <jonatack> Same for me as well
  <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
  ```

  As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.

  It appears that this was broken by bitcoin#16240 which enforced the size of the arguments to match the listed required arguments.

  To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.

ACKs for top commit:
  laanwj:
    ACK 7d26357
  promag:
    ACK 7d26357.

Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants