Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/test/00_setup_env_native_valgrind.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8
export PACKAGES="valgrind clang llvm libclang-rt-dev python3-zmq libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev"
Comment thread
knst marked this conversation as resolved.
export USE_VALGRIND=1
export NO_DEPENDS=1
export TEST_RUNNER_EXTRA="--exclude rpc_bind,feature_bind_extra --timeout-factor=4" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra --timeout-factor=4" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
export GOAL="install"
# Temporarily pin dwarf 4, until valgrind can understand clang's dwarf 5
export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang-19 CXX=clang++-19 CFLAGS='-gdwarf-4' CXXFLAGS='-gdwarf-4'" # TODO enable GUI
2 changes: 1 addition & 1 deletion ci/test/00_setup_env_s390x.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fi
# Use debian to avoid 404 apt errors
export CONTAINER_NAME=ci_s390x
export RUN_UNIT_TESTS=true
export TEST_RUNNER_EXTRA="--exclude rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
export RUN_FUNCTIONAL_TESTS=true
export GOAL="install"
export BITCOIN_CONFIG="--enable-reduce-exports"
7 changes: 7 additions & 0 deletions contrib/seeds/generate-seeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
<ip>:<port>
[<ipv6>]:<port>
<onion>.onion:<port>
<i2p>.b32.i2p:<port>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Document the I2P port restriction.

<port> implies that arbitrary ports are valid, but I2P addresses are accepted only with port 0; non-zero ports are rejected by src/test/evo_netinfo_tests.cpp. Document this as <i2p>.b32.i2p:0 to avoid generating invalid seed entries.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contrib/seeds/generate-seeds.py` at line 19, Update the I2P seed format
example in the relevant documentation or generator template from
“<i2p>.b32.i2p:<port>” to “<i2p>.b32.i2p:0”, explicitly reflecting that only
port 0 is valid for I2P addresses.


The output will be two data structures with the peers in binary format:

Expand Down Expand Up @@ -52,6 +53,12 @@ def name_to_bip155(addr):
return (BIP155Network.TORV2, vchAddr)
else:
raise ValueError('Invalid onion %s' % vchAddr)
elif addr.endswith('.b32.i2p'):
vchAddr = b32decode(addr[0:-8] + '====', True)
Comment on lines +56 to +57

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Match the core parser’s case-insensitive suffix handling.

This rejects valid addresses such as ...b32.I2P, while src/netaddress.cpp lowercases the suffix and src/test/net_tests.cpp explicitly covers uppercase I2P addresses.

Proposed fix
-    elif addr.endswith('.b32.i2p'):
+    elif addr.lower().endswith('.b32.i2p'):
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
elif addr.endswith('.b32.i2p'):
vchAddr = b32decode(addr[0:-8] + '====', True)
elif addr.lower().endswith('.b32.i2p'):
vchAddr = b32decode(addr[0:-8] + '====', True)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contrib/seeds/generate-seeds.py` around lines 56 - 57, Update the address
parsing branch in the seed generator to recognize the .b32.i2p suffix
case-insensitively, matching the core parser’s behavior and accepting uppercase
variants such as .b32.I2P while preserving the existing base32 decoding logic.

if len(vchAddr) == 32:
return (BIP155Network.I2P, vchAddr)
else:
raise ValueError(f'Invalid I2P {vchAddr}')
elif '.' in addr: # IPv4
return (BIP155Network.IPV4, bytes((int(x) for x in addr.split('.'))))
elif ':' in addr: # IPv6 or CJDNS
Expand Down
31 changes: 2 additions & 29 deletions contrib/valgrind.supp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#
# Note that suppressions may depend on OS and/or library versions.
# Tested on:
# * aarch64 (Ubuntu 20.04 system libs, without gui)
# * x86_64 (Ubuntu 18.04 system libs, without gui)
# * aarch64 (Ubuntu 22.04 system libs, clang, without gui)
# * x86_64 (Ubuntu 22.04 system libs, clang, without gui)
{
Suppress libstdc++ warning - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65434
Memcheck:Leak
Expand Down Expand Up @@ -91,33 +91,6 @@
...
fun:GetCoin
}
{
Suppress wcsnrtombs glibc SSE4 warning (could be related: https://stroika.atlassian.net/browse/STK-626)
Memcheck:Addr16
fun:__wcsnlen_sse4_1
fun:wcsnrtombs
}
{
Suppress boost warning
Memcheck:Leak
fun:_Znwm
...
fun:_ZN5boost9unit_test9framework5state17execute_test_treeEmjPKNS2_23random_generator_helperE
fun:_ZN5boost9unit_test9framework3runEmb
fun:_ZN5boost9unit_test14unit_test_mainEPFbvEiPPc
fun:main
}
{
Suppress boost still reachable memory warning
Memcheck:Leak
match-leak-kinds: reachable
fun:_Znwm
...
fun:_M_construct_aux<char*>
fun:_M_construct<char*>
fun:basic_string
fun:path
}
{
Suppress LogInstance still reachable memory warning
Memcheck:Leak
Expand Down
3 changes: 2 additions & 1 deletion doc/bips.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ Versions and PRs are relevant to Bitcoin's core if not mentioned other.
* [`BIP 147`](https://github.com/bitcoin/bips/blob/master/bip-0147.mediawiki): NULLDUMMY softfork as of **v0.13.1** ([PR 8636](https://github.com/bitcoin/bitcoin/pull/8636) and [PR 8937](https://github.com/bitcoin/bitcoin/pull/8937)), *buried* since **v0.19.0** ([PR #16060](https://github.com/bitcoin/bitcoin/pull/16060)).
* [`BIP 152`](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki): Compact block transfer and related optimizations are used as of **v0.13.0** ([PR 8068](https://github.com/bitcoin/bitcoin/pull/8068)).
* [`BIP 155`](https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki): The 'addrv2' and 'sendaddrv2' messages which enable relay of Tor V3 addresses (and other networks) are supported as of **v18.0** ([PR 19954](https://github.com/bitcoin/bitcoin/pull/19954)).
* [`BIP 158`](https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki): Compact Block Filters for Light Clients can be indexed as of **Dash Core v18.0** ([PR dash#4314](https://github.com/dashpay/dash/pull/4314), [PR #14121](https://github.com/bitcoin/bitcoin/pull/14121)).
* [`BIP 157`](https://github.com/bitcoin/bips/blob/master/bip-0157.mediawiki)
[`158`](https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki): Compact Block Filters for Light Clients can be indexed as of **Dash Core v18.0** ([PR dash#4314](https://github.com/dashpay/dash/pull/4314)), ([PR #14121](https://github.com/bitcoin/bitcoin/pull/14121)) and served to peers on the P2P network as of **v0.21.0** ([PR #16442](https://github.com/bitcoin/bitcoin/pull/16442)).
* [`BIP 159`](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki): The `NODE_NETWORK_LIMITED` service bit is signalled as of **v0.16.0** ([PR 11740](https://github.com/bitcoin/bitcoin/pull/11740)), and such nodes are connected to as of **v0.17.0** ([PR 10387](https://github.com/bitcoin/bitcoin/pull/10387)).
* [`BIP 174`](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki): RPCs to operate on Partially Signed Bitcoin Transactions (PSBT) are present as of **v18.0** ([PR 13557](https://github.com/bitcoin/bitcoin/pull/13557)).
* [`BIP 324`](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki): The v2 transport protocol specified by BIP324 and the associated `NODE_P2P_V2` service bit are supported as of **v22.0**, but off by default ([PR 28331](https://github.com/bitcoin/bitcoin/pull/28331)). On by default as of **v22.1** ([PR 29347](https://github.com/bitcoin/bitcoin/pull/29347)).
Expand Down
10 changes: 10 additions & 0 deletions doc/release-notes-16807.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Updated RPCs
------------

- The `validateaddress` RPC now returns an `error_locations` array for invalid
addresses, with the indices of invalid character locations in the address (if
known). For example, this will attempt to locate up to two Bech32 errors, and
return their locations if successful. Success and correctness are only guaranteed
if fewer than two substitution errors have been made.
The error message returned in the `error` field now also returns more specific
errors when decoding fails.
49 changes: 40 additions & 9 deletions src/key_io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,16 @@ class DestinationEncoder
std::string operator()(const CNoDestination& no) const { return {}; }
};

CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str)
CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str, std::vector<int>* error_locations)
{
std::vector<unsigned char> data;
uint160 hash;
error_str = "";
if (DecodeBase58Check(str, data, 21)) {

// Note this will be false if it is a valid Bech32 address for a different network
bool is_bech32 = (ToLower(str.substr(0, params.Bech32PlatformHRP().size())) == params.Bech32PlatformHRP());

if (!is_bech32 && DecodeBase58Check(str, data, 21)) {
// base58-encoded Dash addresses.
// Public-key-hash-addresses have version 76 (or 140 testnet).
// The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key.
Expand All @@ -61,12 +65,39 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
return ScriptHash(hash);
}

// Set potential error message.
error_str = "Invalid prefix for Base58-encoded address";
// If the prefix of data matches either the script or pubkey prefix, the length must have been wrong
if ((data.size() >= script_prefix.size() &&
std::equal(script_prefix.begin(), script_prefix.end(), data.begin())) ||
(data.size() >= pubkey_prefix.size() &&
std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin()))) {
error_str = "Invalid length for Base58 address";
} else {
error_str = "Invalid prefix for Base58-encoded address";
}
return CNoDestination();
} else if (!is_bech32) {
// Try Base58 decoding without the checksum, using a much larger max length
if (!DecodeBase58(str, data, 100)) {
error_str = "Not a valid Bech32m or Base58 encoding";
} else {
error_str = "Invalid checksum or length of Base58 address";
}
return CNoDestination();
}
// Set error message if address can't be interpreted as Base58.
if (error_str.empty()) error_str = "Invalid address format";

// Dash has no Bech32m encoding for L1 destinations: a string using the Platform
// HRP can only be a DIP-18 Platform address, which never encodes an L1 destination.
// Decode it anyway to tell the user why exactly it got rejected.
std::string platform_error_str;
const bool is_platform = IsValidPlatformDestination(DecodePlatformDestination(str, params, platform_error_str));

// Perform Bech32 error location
auto res = bech32::LocateErrors(str);
error_str = res.first;
if (error_locations) *error_locations = std::move(res.second);
if (error_str.empty()) {
error_str = is_platform ? "This is a Dash Platform address, not a Dash Core address" : platform_error_str;
}
return CNoDestination();
}
} // namespace
Expand Down Expand Up @@ -156,9 +187,9 @@ std::string EncodeDestination(const CTxDestination& dest)
return std::visit(DestinationEncoder(Params()), dest);
}

CTxDestination DecodeDestination(const std::string& str, std::string& error_msg)
CTxDestination DecodeDestination(const std::string& str, std::string& error_msg, std::vector<int>* error_locations)
{
return DecodeDestination(str, Params(), error_msg);
return DecodeDestination(str, Params(), error_msg, error_locations);
}

CTxDestination DecodeDestination(const std::string& str)
Expand All @@ -170,7 +201,7 @@ CTxDestination DecodeDestination(const std::string& str)
bool IsValidDestinationString(const std::string& str, const CChainParams& params)
{
std::string error_msg;
return IsValidDestination(DecodeDestination(str, params, error_msg));
return IsValidDestination(DecodeDestination(str, params, error_msg, nullptr));
}

bool IsValidDestinationString(const std::string& str)
Expand Down
2 changes: 1 addition & 1 deletion src/key_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ std::string EncodeExtPubKey(const CExtPubKey& extpubkey);

std::string EncodeDestination(const CTxDestination& dest);
CTxDestination DecodeDestination(const std::string& str);
CTxDestination DecodeDestination(const std::string& str, std::string& error_msg);
CTxDestination DecodeDestination(const std::string& str, std::string& error_msg, std::vector<int>* error_locations = nullptr);
bool IsValidDestinationString(const std::string& str);
bool IsValidDestinationString(const std::string& str, const CChainParams& params);

Expand Down
2 changes: 1 addition & 1 deletion src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ const RPCResult getblock_vin{
{RPCResult::Type::STR, "desc", "Inferred descriptor for the output"},
{RPCResult::Type::STR_HEX, "hex", "The raw public key script bytes, hex-encoded"},
{RPCResult::Type::STR, "address", /*optional=*/true, "The Dash address (only if a well-defined address exists)"},
{RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
{RPCResult::Type::STR, "type", "The output type (e.g. " + GetAllOutputTypes() + ")"},
}},
}},
}},
Expand Down
10 changes: 9 additions & 1 deletion src/rpc/output_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ static RPCHelpMan validateaddress()
{RPCResult::Type::STR_HEX, "scriptPubKey", /*optional=*/true, "The hex-encoded scriptPubKey generated by the address"},
{RPCResult::Type::BOOL, "isscript", /*optional=*/true, "If the key is a script"},
{RPCResult::Type::STR, "error", /*optional=*/true, "Error message, if any"},
{RPCResult::Type::ARR, "error_locations", /*optional=*/true, "Indices of likely error locations in address, if known (e.g. Bech32 errors)",
{
{RPCResult::Type::NUM, "index", "index of a potential error"},
}},
}
},
RPCExamples{
Expand All @@ -48,7 +52,8 @@ static RPCHelpMan validateaddress()
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
std::string error_msg;
CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg);
std::vector<int> error_locations;
CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg, &error_locations);
const bool isValid = IsValidDestination(dest);
CHECK_NONFATAL(isValid == error_msg.empty());

Expand All @@ -64,6 +69,9 @@ static RPCHelpMan validateaddress()
UniValue detail = DescribeAddress(dest);
ret.pushKVs(detail);
} else {
UniValue error_indices(UniValue::VARR);
for (int i : error_locations) error_indices.push_back(i);
ret.pushKV("error_locations", error_indices);
ret.pushKV("error", error_msg);
}

Expand Down
5 changes: 5 additions & 0 deletions src/test/fuzz/script_format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <chainparams.h>
#include <consensus/consensus.h>
#include <core_io.h>
#include <policy/policy.h>
#include <script/script.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
Expand All @@ -19,6 +21,9 @@ FUZZ_TARGET(script_format, .init = initialize_script_format)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
const CScript script{ConsumeScript(fuzzed_data_provider)};
if (script.size() > MAX_STANDARD_TX_SIZE) {
return;
}

(void)FormatScript(script);
(void)ScriptToAsmStr(script, /*fAttemptSighashDecode=*/fuzzed_data_provider.ConsumeBool());
Expand Down
18 changes: 15 additions & 3 deletions src/test/versionbits_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <consensus/params.h>
#include <test/util/random.h>
#include <test/util/setup_common.h>
#include <validation.h>
#include <versionbits.h>

#include <boost/test/unit_test.hpp>
Expand Down Expand Up @@ -185,7 +184,7 @@ class VersionBitsTester
CBlockIndex* Tip() { return vpblock.empty() ? nullptr : vpblock.back(); }
};

BOOST_FIXTURE_TEST_SUITE(versionbits_tests, TestingSetup)
BOOST_FIXTURE_TEST_SUITE(versionbits_tests, BasicTestingSetup)

BOOST_AUTO_TEST_CASE(versionbits_test)
{
Expand Down Expand Up @@ -420,9 +419,22 @@ static void check_computeblockversion(VersionBitsCache& versionbitscache, const
BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1<<bit), 0);
}

/** ComputeBlockVersion() consults the EHF manager global for EHF deployments, which are not exercised here. */
class EHFManagerStub : public AbstractEHFManager
{
public:
EHFManagerStub() {
assert(globalInstance == nullptr);
globalInstance = this;
}
~EHFManagerStub() { globalInstance = nullptr; }
Signals GetSignalsStage(const CBlockIndex* const pindexPrev) override { return {}; }
};

BOOST_AUTO_TEST_CASE(versionbits_computeblockversion)
{
VersionBitsCache vbcache; // don't use chainman versionbitscache since we want custom chain params
EHFManagerStub ehf_manager;
VersionBitsCache vbcache;

// check that any deployment on any chain can conceivably reach both
// ACTIVE and FAILED states in roughly the way we expect
Expand Down
10 changes: 1 addition & 9 deletions src/wallet/rpc/coins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,13 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
bool fAddLocked = (!params[2].isNull() && params[2].get_bool());
const bool include_immature_coinbase{params[3].isNull() ? false : params[3].get_bool()};

// Excluding coinbase outputs is deprecated
// It can be enabled by setting deprecatedrpc=exclude_coinbase
const bool include_coinbase{!wallet.chain().rpcEnableDeprecated("exclude_coinbase")};

if (include_immature_coinbase && !include_coinbase) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "include_immature_coinbase is incompatible with deprecated exclude_coinbase");
}

// Tally
CAmount amount = 0;
for (const std::pair<const uint256, CWalletTx>& wtx_pair : wallet.mapWallet) {
const CWalletTx& wtx = wtx_pair.second;
int depth{wallet.GetTxDepthInMainChain(wtx)};
if (// Coinbase with less than 1 confirmation is no longer in the main chain
(wtx.IsCoinBase() && (depth < 1 || !include_coinbase))
(wtx.IsCoinBase() && (depth < 1))
|| (wallet.IsTxImmatureCoinBase(wtx) && !include_immature_coinbase))
{
continue;
Expand Down
10 changes: 1 addition & 9 deletions src/wallet/rpc/transactions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,6 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
filtered_address = DecodeDestination(params[4].get_str());
}

// Excluding coinbase outputs is deprecated
// It can be enabled by setting deprecatedrpc=exclude_coinbase
const bool include_coinbase{!wallet.chain().rpcEnableDeprecated("exclude_coinbase")};

if (include_immature_coinbase && !include_coinbase) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "include_immature_coinbase is incompatible with deprecated exclude_coinbase");
}

// Tally
std::map<CTxDestination, tallyitem> mapTally;
for (const std::pair<const uint256, CWalletTx>& pairWtx : wallet.mapWallet) {
Expand All @@ -113,7 +105,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
continue;

// Coinbase with less than 1 confirmation is no longer in the main chain
if ((wtx.IsCoinBase() && (nDepth < 1 || !include_coinbase))
if ((wtx.IsCoinBase() && (nDepth < 1))
|| (wallet.IsTxImmatureCoinBase(wtx) && !include_immature_coinbase))
{
continue;
Expand Down
Loading