Age | Commit message (Collapse) | Author |
|
{sign,verify}message RPCs
a5cfb40e27bd281354bd0d14d91f83efb6bfce9f doc: release note for changed {sign,verify}message error codes (Sebastian Falbesoner)
9e399b9b2d386b28c0c0ff59fc75d31dbec31d9c test: check parameter validity in rpc_signmessage.py (Sebastian Falbesoner)
e62f0c71f10def124b1c1219d790cef246a32c3e rpc: fix {sign,message}verify RPC errors for invalid address/signature (Sebastian Falbesoner)
Pull request description:
RPCs that accept address parameters usually return the intended error code `RPC_INVALID_ADDRESS_OR_KEY` (-5) if a passed address is invalid. The two exceptions to the rule are `signmessage` and `verifymessage`, which return `RPC_TYPE_ERROR` (-3) in this case instead. Oddly enough `verifymessage` returns `RPC_INVALID_ADDRESS_OR_KEY` when the _signature_ was malformed, where `RPC_TYPE_ERROR` would be more approriate.
This PR fixes these inaccuracies and as well adds tests to `rpc_signmessage.py` that check the parameter validity and error codes for the related RPCs `signmessagewithprivkey`, `signmessage` and `verifymessage`.
master branch:
```
$ ./bitcoin-cli signmessage invalid_addr message
error code: -3
error message:
Invalid address
$ ./bitcoin-cli verifymessage invalid_addr dummy_sig message
error code: -3
error message:
Invalid address
$ ./bitcoin-cli verifymessage 12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX invalid_sig message
error code: -5
error message:
Malformed base64 encoding
```
PR branch:
```
$ ./bitcoin-cli signmessage invalid_addr message
error code: -5
error message:
Invalid address
$ ./bitcoin-cli verifymessage invalid_addr dummy_sig message
error code: -5
error message:
Invalid address
$ ./bitcoin-cli verifymessage 12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX invalid_sig message
error code: -3
error message:
Malformed base64 encoding
```
ACKs for top commit:
laanwj:
Code review ACK a5cfb40e27bd281354bd0d14d91f83efb6bfce9f
meshcollider:
utACK a5cfb40e27bd281354bd0d14d91f83efb6bfce9f
Tree-SHA512: bae0c4595a2603cea66090f6033785601837b45fd853052312b3a39d8520566c581994b68f693dd247c22586c638c3b7689c849085cce548cc36b9bf0e119d2d
|
|
Signed-off-by: practicalswift <practicalswift@users.noreply.github.com>
|
|
|
|
This commit addresses #20809.
We add an additional 'error' property in the result of 'validateaddress' in case the address is not valid that gives a short description of why the address in invalid. We also change the error message returned by 'getaddressinfo' in case the address is invalid.
|
|
Checking for fHelp, or the size of the args, is dead code because:
* fHelp is always false (src/qt/test/rpcnestedtests.cpp)
* It is already implicitly called by RPCHelpMan::Check
(src/rpc/mining.cpp, src/rpc/misc.cpp, src/rpc/net.cpp)
|
|
|
|
|
|
|
|
|
|
|
|
HexStr can be called with anything that bas `begin()` and `end()` functions,
so clean up the redundant calls.
|
|
This commit does not change behavior
|
|
faec0638872798b58b9882ee079014555bc8393e log: Use Join() helper when listing log categories (MarcoFalke)
Pull request description:
This removes the global `ListLogCategories` and replaces it with a one-line member function `LogCategoriesString`, which just calls `Join`.
Should be a straightforward refactor to get rid of a few LOC.
ACKs for top commit:
laanwj:
ACK faec0638872798b58b9882ee079014555bc8393e
promag:
ACK faec0638872798b58b9882ee079014555bc8393e, I also think it's fine as it is (re https://github.com/bitcoin/bitcoin/pull/18669#discussion_r412944724).
Tree-SHA512: 2f51f9ce1246eda5630015f3a869e36953c7eb34f311baad576b92d7829e4e88051c6189436271cd0a13732a49698506345b446b98fd28e58edfb5b62169f1c9
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
3ce16ad2f91d1e2edc7e7bdc5a19f72aa8c3e739 refactor: Use psbt forward declaration (Russell Yanofsky)
1dde238f2c21a0cc9bada10a2449cf9c6b2178ad Add ChainClient setMockTime, getWallets methods (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
These changes are needed to set mock times, and get wallet interface pointers correctly when
wallet code is running in a different process from node code in #10102
ACKs for top commit:
MarcoFalke:
re-ACK 3ce16ad2f91d1e2edc7e7bdc5a19f72aa8c3e739 🔙
promag:
Code review ACK 3ce16ad2f91d1e2edc7e7bdc5a19f72aa8c3e739.
Tree-SHA512: 6c093bfcd68adf5858a1aade4361cdb7fb015496673504ac7a93d0bd2595215047184551d6fd526baa27782331cd2819ce45c4cf923b205ce93ac29e485b5dd8
|
|
|
|
|
|
8a2a652e6fab5eb8224beefcc07d9011b61865a8 Remove redundant type information from rpc docs (David O'Callaghan)
Pull request description:
Simple edit of the RPC calls to remove redundant text ("A json object/array ...") from the beginning of help.
Fixes: #18258
Top commit has no ACKs.
Tree-SHA512: cbbf760e0b7b4eda61c40b420ed77f5d878318e37b0eb13e63567212240b2c4ecc15d84030e98075e21c9ae9016539adfd201e5661ea824166a76d335180c32f
|
|
3e32499909ca8127baaa9b40ad113b25ee151bbd Change example addresses to bech32 (Yusuf Sahin HAMZA)
Pull request description:
This is a follow-up PR to #18197 that fixes RPCExamples.
Fixes #18185.
ACKs for top commit:
MarcoFalke:
ACK 3e32499909ca8127baaa9b40ad113b25ee151bbd
jonatack:
ACK 3e32499
Tree-SHA512: c7a6410ef8b6e169016c2c5eac3e6b9501caabd0e8a0871ec31e56bfc44589f056d3f5cb55b5a13bba36f6c15136c2352f883e30e4dcc0997ffd36b27f9173b9
|
|
Changes from boost::chrono to std::chrono, boost::condition_var to
std::condition_var, boost::mutex to sync.h Mutex, and reverselock.h to
sync.h REVERSE_LOCK. Also adds threadsafety annotations to CScheduler
members.
|
|
2455aa5d7f54befeade05795ed8f5dd89d01042a [rpc] changed MineBlocksOnDemand to IsMockableChain (Gloria Zhao)
Pull request description:
Change: Update the if statement in `setmocktime` to use `IsMockableChain` chainparams function (aka `m_is_mockable_chain`) instead of `MineBlocksOnDemand`
Rationale: It's a more appropriate check for whether or not chain is in RegTest, as [discussed](https://github.com/bitcoin/bitcoin/pull/18037#discussion_r376509388) in #18037
ACKs for top commit:
MarcoFalke:
ACK 2455aa5d7f54befeade05795ed8f5dd89d01042a 🙇
jonatack:
ACK 2455aa5d7f54befeade05795ed8f5dd89d01042a
Tree-SHA512: 1d8c8b7ff0b3c1bcbf5755194969b6664fe05a35003375ad08d18e34bcefd2df4f64d0e60078a10bbef3c8f469a9b9d07db467089b55c14cf532304bc965bffc
|
|
|
|
Co-Authored-By: MarcoFalke <falke.marco@gmail.com>
|
|
|
|
|
|
Needed to set mock times, and get wallet interface pointers correctly when
wallet code is running in a different process from node code.
|
|
e193a84fb28068e38d5f54fbfd6208428c5bb655 Refactor message hashing into a utility function (Jeffrey Czyz)
f8f0d9893d7969bdaa870fadb94ec5d0dfa8334d Deduplicate the message signing code (Vasil Dimov)
2ce3447eb1e25ec7aec4b300dabf6c1e394f1906 Deduplicate the message verifying code (Vasil Dimov)
Pull request description:
The message signing and verifying logic was replicated in a few places
in the code. Consolidate in a newly introduced `MessageSign()` and
`MessageVerify()` and add unit tests for them.
ACKs for top commit:
Sjors:
re-ACK e193a84fb28068e38d5f54fbfd6208428c5bb655
achow101:
ACK e193a84fb28068e38d5f54fbfd6208428c5bb655
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/17577/commits/e193a84fb28068e38d5f54fbfd6208428c5bb655
meshcollider:
utACK e193a84fb28068e38d5f54fbfd6208428c5bb655
Tree-SHA512: b0e02a7d4623a98c8f8c77627af1725e6df07700de4630c2f75da6beacdf55414c38ba147bc6d2a757491ab07c827dddf93e8632fe600478760e255714ddab88
|
|
7f1475c7119e8c72bce39a63386a6ca859066b80 rpc: update validateaddress RPCExamples to bech32 (Sebastian Falbesoner)
Pull request description:
Another small step to get rid of legacy addresses in the RPC help texts and by that encourage the use of bech32 addresses by default. The (invalid) address is the same as in the `getaddressinfo` RPC (see 2ee0cb3330ccf70f0540cb42370796e32eff1569, kudos to jonatack!), I don't think it adds any value to have a different example address per RPC.
ACKs for top commit:
fanquake:
ACK 7f1475c7119e8c72bce39a63386a6ca859066b80
MarcoFalke:
ACK 7f1475c7119e8c72bce39a63386a6ca859066b80
Tree-SHA512: 2350f61fa942a9053f9f5c860ea446965dc7209c71c81bdb98a859d03ca23b225ad72c9c506e4a55c8d8988823d9cfbe808c1a452a1eeadb70ab186b146dd4ca
|
|
8bca30ea17cd4c1dacee28eaa27e5fa3493b021d [rpc] expose ability to mock scheduler via the rpc (Amiti Uttarwar)
7c8b6e5b5206a98f86675d0107ad99ea1d080466 [lib] add scheduler to node context (Amiti Uttarwar)
930d8375421451c8c4127608c360b0f6a0a62127 [test] add chainparams property to indicate chain allows time mocking (Amiti Uttarwar)
1cd43e83c6e8d81e950aaaede7a8a51505d0a2bc [test] unit test for new MockForward scheduler method (Amiti Uttarwar)
a6f63598adb880a75e1571aac58338c17fa7ad53 [util] allow scheduler to be mocked (Amiti Uttarwar)
Pull request description:
This PR is to support functional tests by allowing the scheduler to be mocked via the RPC.
It adds a `MockForward` method to the scheduler class that iterates through the task queue and reschedules them to be `delta_seconds` sooner.
This is currently used to support functional testing of the "unbroadcast" set tracking in #18038. If this patch is accepted, it would also be useful to simplify the code in #16698.
ACKs for top commit:
MarcoFalke:
ACK 8bca30ea17cd4c1dacee28eaa27e5fa3493b021d, only change is some style fixups 🕓
Tree-SHA512: 2a97fe8ade2b7fd1fb5cdfa1dcafb3227a377d7a847e3845a228bc119eb77824b4aefa43d922a06d583939b22725e223f308cf092961048079d36f6b1d9a639b
|
|
|
|
The logic of signing a message was duplicated in 3 places:
src/qt/signverifymessagedialog.cpp
SignVerifyMessageDialog::on_signMessageButton_SM_clicked()
src/rpc/misc.cpp
signmessagewithprivkey()
src/wallet/rpcwallet.cpp
signmessage()
Move the logic into
src/util/message.cpp
MessageSign()
and call it from all the 3 places.
|
|
The logic of verifying a message was duplicated in 2 places:
src/qt/signverifymessagedialog.cpp
SignVerifyMessageDialog::on_verifyMessageButton_VM_clicked()
src/rpc/misc.cpp
verifymessage()
with the only difference being the result handling. Move the logic into
a dedicated
src/util/message.cpp
MessageVerify()
which returns a set of result codes, call it from the 2 places and just
handle the results differently in the callers.
|
|
also contains the following changes:
- rpc: factor out example bech32 address for RPCExamples
- doc: update developer notes wrt RPCExamples addresses
(mention the EXAMPLE_ADDRESS constant as an example for an invalid bech32
address suitable for RPCExamples help documentation)
|
|
This makes the rendered diff smaller when the RPCResult is machine
generated later on
-BEGIN VERIFY SCRIPT-
# Add space after dictionary key and before colon
sed -i --regexp-extended -e 's/(^ +" +\\"[a-zA-Z_]+\\"): ?/\1 : /g' $(git grep -l '\\":')
# Rename (array) to (json array)
sed -i -e 's/ (array) / (json array) /g' $(git grep -l '(array)' ./src)
# Rename (object) to (json object)
sed -i -e 's/ (object) / (json object) /g' $(git grep -l '(object)' ./src)
# Rename (bool) to (boolean)
sed -i -e 's/ (bool) / (boolean) /g' $(git grep -l '(bool)' ./src)
# Rename (int) to (numeric)
sed -i -e 's/ (int) / (numeric) /g' $(git grep -l '(int)' ./src)
-END VERIFY SCRIPT-
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
to "UNIX epoch time".
Call sites updated:
```
mocktime
getblockheader
getblock
pruneblockchain
getchaintxstats
getblocktemplate
setmocktime
getpeerinfo
setban
getnodeaddresses
getrawtransaction
importmulti
listtransactions
listsinceblock
gettransaction
getwalletinfo
getaddressinfo
```
|
|
|
|
|
|
Some failure conditions implicitly fail by failing some other check.
But the error messages are more helpful if they say explicitly what
actually caused the failure, so add those as failure conditions and
errors.
|
|
failed
|
|
|
|
93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9 Move WatchOnly stuff from SigningProvider to CWallet (Andrew Chow)
8f5b81e6edae9cb22559545de63f391d97c15701 Remove CCryptoKeyStore and move all of it's functionality into CWallet (Andrew Chow)
37a79a4fccbf6cd65a933594e24e59d36e674653 Move various SigningProviders to signingprovider.{cpp,h} (Andrew Chow)
16f8096e911e4d59292240a17e2d4004f0500b9e Move KeyOriginInfo to its own header file (Andrew Chow)
d9becff4e13da8e182631baa79b9794c03d44434 scripted-diff: rename CBasicKeyStore to FillableSigningProvider (Andrew Chow)
a913e3f2fbeb1352fc66f334d4f5f7332ea89ad7 Move HaveKey static function from keystore to rpcwallet where it is used (Andrew Chow)
c7797ec65544bd23a2e571b2892e1bf512f2a485 Remove CKeyStore and squash into CBasicKeyStore (Andrew Chow)
1b699a5083b435c2b79f3951f94ac9f967d24f6c Add HaveKey and HaveCScript to SigningProvider (Andrew Chow)
Pull request description:
This PR compresses the `CWallet` chain of inheritance from 5 classes to 3 classes. `CBasicKeyStore` is renamed to `FillableSigningProvider` and some parts of it (the watchonly parts) are moved into `CWallet`. `CKeyStore` and `CCrypoKeyStore` are completely removed. `CKeyStore`'s `Have*` functions are moved into `SigningProvider` and the `Add*` moved into `FillableSigningProvider`, thus allowing it to go away entirely. `CCryptoKeyStore`'s functionality is moved into `CWallet`. The new inheritance chain is:
```
SigningProvider -> FillableSigningProvider -> CWallet
```
`SigningProvider` now is the class the provides keys and scripts and indicates whether keys and scripts are present. `FillableSigningProvider` allows keys and scripts to be added to the signing provider via `Add*` functions. `CWallet` handles all of the watchonly stuff (`AddWatchOnly`, `HaveWatchOnly`, `RemoveWatchOnly` which were previously in `CKeyStore`) and key encryption (previously in `CCryptoKeyStore`).
Implements the 2nd [prerequisite](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#cwallet-subclass-stack) from the wallet restructure.
ACKs for top commit:
Sjors:
re-ACK 93ce4a0; it keeps `EncryptSecret`, `DecryptSecret` and `DecryptKey` in `wallet/crypter.cpp`, but makes them not static. It improves alphabetical includes, reorders some function definitions, fixes commit message, brings back lost code comment.
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/16227/commits/93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9
Tree-SHA512: 393dfd0623ad2dac38395eb89b862424318d6072f0b7083c92a0d207fd032c48b284f5f2cb13bc492f34557de350c5fee925da02e47daf011c5c6930a721b6d3
|
|
-BEGIN VERIFY SCRIPT-
git grep -l "CBasicKeyStore" | xargs sed -i -e 's/CBasicKeyStore/FillableSigningProvider/g'
-END VERIFY SCRIPT-
|
|
|
|
returns a legacy address
a49503402b6bc21e3878e151c07529941d36aed0 Make and get the multisig redeemscript and destination in one function instead of two (Andrew Chow)
Pull request description:
`CreateMultisigRedeemscript()` is changed to `AddAndGetMultisigDestination()` so that the process of constructing the redeemScript and then getting the `CTxDestination` are done in the same function. This allows that function to see what the keys in the multisig are so that the correct address type is returned from `AddAndGetDestinationForScript()`.
This only effects the `createmultisig` and `addmultisigaddress` RPCs and does not change signing logic as #16022 does.
Alternative to #16022 and #16012
Fixes #16011
ACKs for commit a49503:
Tree-SHA512: 5b0154a714deea3b2cc3a54beb420c95eeeacf4ca30c40ca80940d9d640f8b03611b0fc14c2f0710bfd8a79e8d27ad7d9ae380b4b83d52b40ab201624f2a63f0
|
|
instead of two
Instead of creating a redeemScript with CreateMultisigRedeemscript and
then getting the destination with AddAndGetDestinationForScript, do
both in the same function.
CreateMultisigRedeemscript is changed to AddAndGetMultisigDestination.
It creates the redeemScript and returns it via an output parameter. Then
it calls AddAndGetDestinationForScript to add the destination to the
keystore and get the proper destination.
This allows us to inspect the public keys in the redeemScript before creating
the destination so that the correct destination is used when uncompressed
pubkeys are in the multisig.
|
|
|
|
|