Skip to content

Commit 1c7be9a

Browse files
author
MarcoFalke
committed
Merge #20286: rpc: deprecate addresses and reqSigs from rpc outputs
90ae3d8 doc: Add release notes for -deprecatedrpc=addresses and bitcoin-tx (Michael Dietz) 085b3a7 rpc: deprecate `addresses` and `reqSigs` from rpc outputs (Michael Dietz) Pull request description: Considering the limited applicability of `reqSigs` and the confusing output of `1` in all cases except bare multisig, the `addresses` and `reqSigs` outputs are removed for all rpc commands. 1) add a new sane "address" field (for outputs that have an identifiable address, which doesn't include bare multisig) 2) with -deprecatedrpc: leave "reqSigs" and "addresses" intact (with all weird/wrong behavior they have now) 3) without -deprecatedrpc: drop "reqSigs" and "addresses" entirely always. Note: Some light refactoring done to allow us to very easily delete a few chunks of code (marked with TODOs) when we remove this deprecated behavior. Using `IsDeprecatedRPCEnabled` in core_write.cpp caused some circular dependencies involving core_io Circular dependencies were caused by rpc/util unnecessarily importing node/coinstats and node/transaction. Really what rpc/util needs are some fundamental type/helper-function definitions. So this was cleaned up to make more sense. This fixes #20102. ACKs for top commit: MarcoFalke: re-ACK 90ae3d8 📢 Tree-SHA512: 8ffb617053b5f4a8b055da17c06711fd19632e0037d71c4c8135e50c8cd7a19163989484e4e0f17a6cc48bd597f04ecbfd609aef54b7d1d1e76a784214fcf72a
2 parents 4399dc8 + 90ae3d8 commit 1c7be9a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+213
-183
lines changed

‎doc/REST-interface.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,8 @@ $ curl localhost:18332/rest/getutxos/checkmempool/b2cdfd7b89def827ff8af7cd9bff76
9595
"scriptPubKey" : {
9696
"asm" : "OP_DUP OP_HASH160 1c7cebb529b86a04c683dfa87be49de35bcf589e OP_EQUALVERIFY OP_CHECKSIG",
9797
"hex" : "76a9141c7cebb529b86a04c683dfa87be49de35bcf589e88ac",
98-
"reqSigs" : 1,
9998
"type" : "pubkeyhash",
100-
"addresses" : [
101-
"mi7as51dvLJsizWnTMurtRmrP8hG2m1XvD"
102-
]
99+
"address" : "mi7as51dvLJsizWnTMurtRmrP8hG2m1XvD"
103100
}
104101
}
105102
]

‎doc/release-notes.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,21 @@ Updated RPCs
6767
`whitelisted`, the `permissions` field indicates if the peer has special
6868
privileges. The `banscore` field has simply been removed. (#20755)
6969

70+
- The following RPCs: `gettxout`, `getrawtransaction`, `decoderawtransaction`,
71+
`decodescript`, `gettransaction`, and REST endpoints: `/rest/tx`,
72+
`/rest/getutxos`, `/rest/block` deprecated the following fields (which are no
73+
longer returned in the responses by default): `addresses`, `reqSigs`.
74+
The `-deprecatedrpc=addresses` flag must be passed for these fields to be
75+
included in the RPC response. This flag/option will be available until v23, at which
76+
point the deprecation will be removed entirely. Note that these fields are attributes of
77+
the `scriptPubKey` object returned in the RPC response. However, in the response
78+
of `decodescript` these fields are top-level attributes, and included again as attributes
79+
of the `scriptPubKey` object. (#20286)
80+
81+
- When creating a hex-encoded bitcoin transaction using the `bitcoin-tx` utility
82+
with the `-json` option set, the following fields: `addresses`, `reqSigs` are no longer
83+
returned in the tx output of the response. (#20286)
84+
7085
Changes to Wallet or GUI related RPCs can be found in the GUI or Wallet section below.
7186

7287
New RPCs

‎src/bitcoin-tx.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ static void MutateTx(CMutableTransaction& tx, const std::string& command,
725725
static void OutputTxJSON(const CTransaction& tx)
726726
{
727727
UniValue entry(UniValue::VOBJ);
728-
TxToUniv(tx, uint256(), entry);
728+
TxToUniv(tx, uint256(), /* include_addresses */ false, entry);
729729

730730
std::string jsonOutput = entry.write(4);
731731
tfm::format(std::cout, "%s\n", jsonOutput);

‎src/core_io.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ UniValue ValueFromAmount(const CAmount amount);
4444
std::string FormatScript(const CScript& script);
4545
std::string EncodeHexTx(const CTransaction& tx, const int serializeFlags = 0);
4646
std::string SighashToStr(unsigned char sighash_type);
47-
void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex);
47+
void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex, bool include_addresses);
4848
void ScriptToUniv(const CScript& script, UniValue& out, bool include_address);
49-
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr);
49+
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr);
5050

5151
#endif // BITCOIN_CORE_IO_H

‎src/core_write.cpp

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,13 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_address)
156156
}
157157
}
158158

159+
// TODO: from v23 ("addresses" and "reqSigs" deprecated) this method should be refactored to remove the `include_addresses` option
160+
// this method can also be combined with `ScriptToUniv` as they will overlap
159161
void ScriptPubKeyToUniv(const CScript& scriptPubKey,
160-
UniValue& out, bool fIncludeHex)
162+
UniValue& out, bool fIncludeHex, bool include_addresses)
161163
{
162164
TxoutType type;
165+
CTxDestination address;
163166
std::vector<CTxDestination> addresses;
164167
int nRequired;
165168

@@ -172,17 +175,22 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey,
172175
return;
173176
}
174177

175-
out.pushKV("reqSigs", nRequired);
178+
if (ExtractDestination(scriptPubKey, address)) {
179+
out.pushKV("address", EncodeDestination(address));
180+
}
176181
out.pushKV("type", GetTxnOutputType(type));
177182

178-
UniValue a(UniValue::VARR);
179-
for (const CTxDestination& addr : addresses) {
180-
a.push_back(EncodeDestination(addr));
183+
if (include_addresses) {
184+
UniValue a(UniValue::VARR);
185+
for (const CTxDestination& addr : addresses) {
186+
a.push_back(EncodeDestination(addr));
187+
}
188+
out.pushKV("addresses", a);
189+
out.pushKV("reqSigs", nRequired);
181190
}
182-
out.pushKV("addresses", a);
183191
}
184192

185-
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo)
193+
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo)
186194
{
187195
entry.pushKV("txid", tx.GetHash().GetHex());
188196
entry.pushKV("hash", tx.GetWitnessHash().GetHex());
@@ -241,7 +249,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
241249
out.pushKV("n", (int64_t)i);
242250

243251
UniValue o(UniValue::VOBJ);
244-
ScriptPubKeyToUniv(txout.scriptPubKey, o, true);
252+
ScriptPubKeyToUniv(txout.scriptPubKey, o, true, include_addresses);
245253
out.pushKV("scriptPubKey", o);
246254
vout.push_back(out);
247255

‎src/rpc/blockchain.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,11 +1113,13 @@ static RPCHelpMan gettxout()
11131113
{RPCResult::Type::NUM, "confirmations", "The number of confirmations"},
11141114
{RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT},
11151115
{RPCResult::Type::OBJ, "scriptPubKey", "", {
1116-
{RPCResult::Type::STR_HEX, "asm", ""},
1116+
{RPCResult::Type::STR, "asm", ""},
11171117
{RPCResult::Type::STR_HEX, "hex", ""},
1118-
{RPCResult::Type::NUM, "reqSigs", "Number of required signatures"},
1119-
{RPCResult::Type::STR_HEX, "type", "The type, eg pubkeyhash"},
1120-
{RPCResult::Type::ARR, "addresses", "array of bitcoin addresses", {{RPCResult::Type::STR, "address", "bitcoin address"}}},
1118+
{RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
1119+
{RPCResult::Type::STR, "type", "The type, eg pubkeyhash"},
1120+
{RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"},
1121+
{RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses",
1122+
{{RPCResult::Type::STR, "address", "bitcoin address"}}},
11211123
}},
11221124
{RPCResult::Type::BOOL, "coinbase", "Coinbase or not"},
11231125
}},
@@ -1775,6 +1777,16 @@ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES],
17751777
}
17761778
}
17771779

1780+
void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex)
1781+
{
1782+
ScriptPubKeyToUniv(scriptPubKey, out, fIncludeHex, IsDeprecatedRPCEnabled("addresses"));
1783+
}
1784+
1785+
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo)
1786+
{
1787+
TxToUniv(tx, hashBlock, IsDeprecatedRPCEnabled("addresses"), entry, include_hex, serialize_flags, txundo);
1788+
}
1789+
17781790
template<typename T>
17791791
static inline bool SetHasKeys(const std::set<T>& set) {return false;}
17801792
template<typename T, typename Tk, typename... Args>

‎src/rpc/blockchain.h

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

88
#include <amount.h>
9+
#include <core_io.h>
910
#include <streams.h>
1011
#include <sync.h>
1112

@@ -54,6 +55,9 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex
5455
/** Used by getblockstats to get feerates at different percentiles by weight */
5556
void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_weight);
5657

58+
void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex);
59+
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr);
60+
5761
NodeContext& EnsureNodeContext(const util::Ref& context);
5862
CTxMemPool& EnsureMemPool(const util::Ref& context);
5963
ChainstateManager& EnsureChainman(const util::Ref& context);

‎src/rpc/rawtransaction.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
#include <validation.h>
3636
#include <validationinterface.h>
3737

38-
3938
#include <numeric>
4039
#include <stdint.h>
4140

@@ -132,9 +131,10 @@ static RPCHelpMan getrawtransaction()
132131
{
133132
{RPCResult::Type::STR, "asm", "the asm"},
134133
{RPCResult::Type::STR, "hex", "the hex"},
135-
{RPCResult::Type::NUM, "reqSigs", "The required sigs"},
134+
{RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
136135
{RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
137-
{RPCResult::Type::ARR, "addresses", "",
136+
{RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"},
137+
{RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses",
138138
{
139139
{RPCResult::Type::STR, "address", "bitcoin address"},
140140
}},
@@ -490,9 +490,10 @@ static RPCHelpMan decoderawtransaction()
490490
{
491491
{RPCResult::Type::STR, "asm", "the asm"},
492492
{RPCResult::Type::STR_HEX, "hex", "the hex"},
493-
{RPCResult::Type::NUM, "reqSigs", "The required sigs"},
493+
{RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
494494
{RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
495-
{RPCResult::Type::ARR, "addresses", "",
495+
{RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"},
496+
{RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses",
496497
{
497498
{RPCResult::Type::STR, "address", "bitcoin address"},
498499
}},
@@ -548,8 +549,9 @@ static RPCHelpMan decodescript()
548549
{
549550
{RPCResult::Type::STR, "asm", "Script public key"},
550551
{RPCResult::Type::STR, "type", "The output type (e.g. "+GetAllOutputTypes()+")"},
551-
{RPCResult::Type::NUM, "reqSigs", "The required signatures"},
552-
{RPCResult::Type::ARR, "addresses", "",
552+
{RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"},
553+
{RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
554+
{RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses",
553555
{
554556
{RPCResult::Type::STR, "address", "bitcoin address"},
555557
}},
@@ -559,8 +561,9 @@ static RPCHelpMan decodescript()
559561
{RPCResult::Type::STR, "asm", "String representation of the script public key"},
560562
{RPCResult::Type::STR_HEX, "hex", "Hex string of the script public key"},
561563
{RPCResult::Type::STR, "type", "The type of the script public key (e.g. witness_v0_keyhash or witness_v0_scripthash)"},
562-
{RPCResult::Type::NUM, "reqSigs", "The required signatures (always 1)"},
563-
{RPCResult::Type::ARR, "addresses", "(always length 1)",
564+
{RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"},
565+
{RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
566+
{RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses",
564567
{
565568
{RPCResult::Type::STR, "address", "segwit address"},
566569
}},

‎src/script/standard.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,14 +220,14 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
220220
return true;
221221
}
222222
case TxoutType::MULTISIG:
223-
// Multisig txns have more than one address...
224223
case TxoutType::NULL_DATA:
225224
case TxoutType::NONSTANDARD:
226225
return false;
227226
} // no default case, so the compiler can warn about missing cases
228227
assert(false);
229228
}
230229

230+
// TODO: from v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed
231231
bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector<CTxDestination>& addressRet, int& nRequiredRet)
232232
{
233233
addressRet.clear();

‎src/script/standard.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,8 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
247247
* Note: this function confuses destinations (a subset of CScripts that are
248248
* encodable as an address) with key identifiers (of keys involved in a
249249
* CScript), and its use should be phased out.
250+
*
251+
* TODO: from v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed
250252
*/
251253
bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector<CTxDestination>& addressRet, int& nRequiredRet);
252254

0 commit comments

Comments
 (0)