Skip to content

Commit 1ed8a0f

Browse files
achow101achow101
committed
Merge bitcoin/bitcoin#28113: kernel: Remove UniValue from kernel library
6960c81 kernel: Remove Univalue from kernel library (TheCharlatan) 10eb3a9 kernel: Split ParseSighashString (TheCharlatan) Pull request description: Besides the build system changes, this is a mostly move-only change for moving the few UniValue-related functions out of kernel files. UniValue is not required by any of the kernel components and a JSON library should not need to be part of a consensus library. ACKs for top commit: achow101: ACK 6960c81 theuni: Re-ACK 6960c81 stickies-v: re-ACK bitcoin/bitcoin@6960c81 Tree-SHA512: d92e4cb4e12134c94b517751bd746d39f9b8da528ec3a1c94aaedcce93274a3bae9277832e8a7c0243c13df0397ca70ae7bbb24ede200018c569f8d81103c1da
2 parents e35fb7b + 6960c81 commit 1ed8a0f

File tree

8 files changed

+59
-43
lines changed

8 files changed

+59
-43
lines changed

‎doc/release-notes-28113.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
RPC Wallet
2+
----------
3+
4+
- The `signrawtransactionwithkey`, `signrawtransactionwithwallet`,
5+
`walletprocesspsbt` and `descriptorprocesspsbt` calls now return more
6+
specific RPC_INVALID_PARAMETER instead of RPC_PARSE_ERROR if their
7+
sighashtype argument is malformed or not a string.

‎src/Makefile.am

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -898,8 +898,8 @@ if BUILD_BITCOIN_KERNEL_LIB
898898
lib_LTLIBRARIES += $(LIBBITCOINKERNEL)
899899

900900
libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) $(PTHREAD_FLAGS)
901-
libbitcoinkernel_la_LIBADD = $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1)
902-
libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)
901+
libbitcoinkernel_la_LIBADD = $(LIBBITCOIN_CRYPTO) $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1)
902+
libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS)
903903

904904
# libbitcoinkernel requires default symbol visibility, explicitly specify that
905905
# here so that things still work even when user configures with

‎src/bitcoin-tx.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,16 @@ static CAmount AmountFromValue(const UniValue& value)
562562
return amount;
563563
}
564564

565+
static std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName)
566+
{
567+
std::string strHex;
568+
if (v.isStr())
569+
strHex = v.getValStr();
570+
if (!IsHex(strHex))
571+
throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')");
572+
return ParseHex(strHex);
573+
}
574+
565575
static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
566576
{
567577
int nHashType = SIGHASH_ALL;

‎src/core_io.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define BITCOIN_CORE_IO_H
77

88
#include <consensus/amount.h>
9+
#include <util/result.h>
910

1011
#include <string>
1112
#include <vector>
@@ -45,8 +46,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header);
4546
* @see ParseHashV for an RPC-oriented version of this
4647
*/
4748
bool ParseHashStr(const std::string& strHex, uint256& result);
48-
std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName);
49-
int ParseSighashString(const UniValue& sighash);
49+
[[nodiscard]] util::Result<int> SighashFromStr(const std::string& sighash);
5050

5151
// core_write.cpp
5252
UniValue ValueFromAmount(const CAmount amount);

‎src/core_read.cpp

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#include <script/sign.h>
1111
#include <serialize.h>
1212
#include <streams.h>
13-
#include <univalue.h>
13+
#include <util/result.h>
1414
#include <util/strencodings.h>
1515
#include <version.h>
1616

@@ -242,36 +242,21 @@ bool ParseHashStr(const std::string& strHex, uint256& result)
242242
return true;
243243
}
244244

245-
std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName)
245+
util::Result<int> SighashFromStr(const std::string& sighash)
246246
{
247-
std::string strHex;
248-
if (v.isStr())
249-
strHex = v.getValStr();
250-
if (!IsHex(strHex))
251-
throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')");
252-
return ParseHex(strHex);
253-
}
254-
255-
int ParseSighashString(const UniValue& sighash)
256-
{
257-
int hash_type = SIGHASH_DEFAULT;
258-
if (!sighash.isNull()) {
259-
static std::map<std::string, int> map_sighash_values = {
260-
{std::string("DEFAULT"), int(SIGHASH_DEFAULT)},
261-
{std::string("ALL"), int(SIGHASH_ALL)},
262-
{std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)},
263-
{std::string("NONE"), int(SIGHASH_NONE)},
264-
{std::string("NONE|ANYONECANPAY"), int(SIGHASH_NONE|SIGHASH_ANYONECANPAY)},
265-
{std::string("SINGLE"), int(SIGHASH_SINGLE)},
266-
{std::string("SINGLE|ANYONECANPAY"), int(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY)},
267-
};
268-
const std::string& strHashType = sighash.get_str();
269-
const auto& it = map_sighash_values.find(strHashType);
270-
if (it != map_sighash_values.end()) {
271-
hash_type = it->second;
272-
} else {
273-
throw std::runtime_error(strHashType + " is not a valid sighash parameter.");
274-
}
247+
static std::map<std::string, int> map_sighash_values = {
248+
{std::string("DEFAULT"), int(SIGHASH_DEFAULT)},
249+
{std::string("ALL"), int(SIGHASH_ALL)},
250+
{std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)},
251+
{std::string("NONE"), int(SIGHASH_NONE)},
252+
{std::string("NONE|ANYONECANPAY"), int(SIGHASH_NONE|SIGHASH_ANYONECANPAY)},
253+
{std::string("SINGLE"), int(SIGHASH_SINGLE)},
254+
{std::string("SINGLE|ANYONECANPAY"), int(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY)},
255+
};
256+
const auto& it = map_sighash_values.find(sighash);
257+
if (it != map_sighash_values.end()) {
258+
return it->second;
259+
} else {
260+
return util::Error{Untranslated(sighash + " is not a valid sighash parameter.")};
275261
}
276-
return hash_type;
277262
}

‎src/rpc/util.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

55
#include <clientversion.h>
6+
#include <core_io.h>
67
#include <common/args.h>
78
#include <consensus/amount.h>
9+
#include <script/interpreter.h>
810
#include <key_io.h>
911
#include <outputtype.h>
1012
#include <rpc/util.h>
1113
#include <script/descriptor.h>
1214
#include <script/signingprovider.h>
1315
#include <tinyformat.h>
1416
#include <util/check.h>
17+
#include <util/result.h>
1518
#include <util/strencodings.h>
1619
#include <util/string.h>
1720
#include <util/translation.h>
@@ -310,6 +313,21 @@ UniValue DescribeAddress(const CTxDestination& dest)
310313
return std::visit(DescribeAddressVisitor(), dest);
311314
}
312315

316+
int ParseSighashString(const UniValue& sighash)
317+
{
318+
if (sighash.isNull()) {
319+
return SIGHASH_DEFAULT;
320+
}
321+
if (!sighash.isStr()) {
322+
throw JSONRPCError(RPC_INVALID_PARAMETER, "sighash needs to be null or string");
323+
}
324+
const auto result{SighashFromStr(sighash.get_str())};
325+
if (!result) {
326+
throw JSONRPCError(RPC_INVALID_PARAMETER, util::ErrorString(result).original);
327+
}
328+
return result.value();
329+
}
330+
313331
unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target)
314332
{
315333
const int target{value.getInt<int>()};

‎src/rpc/util.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto
100100

101101
UniValue DescribeAddress(const CTxDestination& dest);
102102

103+
/** Parse a sighash string representation and raise an RPC error if it is invalid. */
104+
int ParseSighashString(const UniValue& sighash);
105+
103106
//! Parse a confirm target option and raise an RPC error if it is invalid.
104107
unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target);
105108

‎src/test/fuzz/parse_univalue.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

55
#include <chainparams.h>
6-
#include <core_io.h>
76
#include <rpc/client.h>
87
#include <rpc/util.h>
98
#include <test/fuzz/fuzz.h>
@@ -57,12 +56,6 @@ FUZZ_TARGET(parse_univalue, .init = initialize_parse_univalue)
5756
(void)ParseHexO(univalue, random_string);
5857
} catch (const UniValue&) {
5958
}
60-
try {
61-
(void)ParseHexUV(univalue, "A");
62-
(void)ParseHexUV(univalue, random_string);
63-
} catch (const UniValue&) {
64-
} catch (const std::runtime_error&) {
65-
}
6659
try {
6760
(void)ParseHexV(univalue, "A");
6861
} catch (const UniValue&) {
@@ -75,7 +68,7 @@ FUZZ_TARGET(parse_univalue, .init = initialize_parse_univalue)
7568
}
7669
try {
7770
(void)ParseSighashString(univalue);
78-
} catch (const std::runtime_error&) {
71+
} catch (const UniValue&) {
7972
}
8073
try {
8174
(void)AmountFromValue(univalue);

0 commit comments

Comments
 (0)