Enhanced error messages for invalid network prefix during address parsing. #27260

portlandhodl · 2023-03-15T10:17:16Z

Improve Address Decoding Error Messages

Issue

DecodeDestination does not properly handle errors when a user inputs a valid address for the wrong network (e.g., a testnet address while running the client on mainnet).

The previous error messages were misleading. For example, "Invalid or unsupported Segwit (Bech32) or Base58 encoding" was displayed for a valid Bech32 address on a different network. This occurred because the is_bech32 variable only checked if the prefix matched the current network's HRP. If it didn't match, the code would fall through to Base58 decoding logic regardless of whether the string was actually a valid Bech32 address.

Solution

Implementation Approach

Decode first, then validate: Instead of checking the prefix before decoding, we now decode the string using bech32::Decode(str) upfront. This takes minimal CPU cycles and is acceptable since address validation is not a frequent operation.
Gather decoding information: After Bech32 decoding, we also attempt DecodeBase58 (with a length of 100) and DecodeBase58Check. This provides enough information to properly diagnose errors.
Provide network-aware error messages: When an address has an invalid prefix, the error message now includes the expected network values.

Changes Made

1. Add Base58 Encoded Prefixes to ChainParams (`src/kernel/chainparams.cpp`, `src/kernel/chainparams.h`)

2. Refactor Address Decoding Logic (`src/key_io.cpp`)

Decode Bech32 upfront using bech32::Decode(str) instead of checking prefix first
Use structured bindings for cleaner code: auto [bech32_encoding, bech32_hrp, bech32_chars] = bech32::Decode(str)
Improved error handling flow with clearer branching

3. Improved Error Messages

Scenario	Previous Error	New Error
Base58 address with wrong prefix	"Invalid or unsupported Base58-encoded address."	"Invalid Base58 address. Expected prefix 3, or 1"
Bech32 address with wrong HRP	"Invalid or unsupported Segwit (Bech32) or Base58 encoding."	"Invalid or unsupported prefix for Segwit (Bech32) address (expected bc, got tb)"
Invalid Base58 checksum	"Invalid or unsupported Segwit (Bech32) or Base58 encoding."	"Invalid checksum or length of Base58 address (P2PKH or P2SH)"
Ambiguous invalid address	"Invalid or unsupported Segwit (Bech32) or Base58 encoding."	"Address is not valid Base58 or Bech32"
Bech32 with checksum error	N/A	"Bech32 address decoded with error: Invalid Bech32 checksum"

4. Test Updates

Added test/functional/data/rpc_validateaddress.json with comprehensive test vectors for both mainnet and signet
Updated test/functional/rpc_validateaddress.py to use data-driven testing across multiple networks
Updated test/functional/rpc_invalid_address_message.py with new error message expectations

Consistency Improvements

Removed inconsistent periods at the end of some error messages
Changed "Bech32 v0" to "SegWit v0" for clarity

Reference

Issue: Improve the error message when an address cannot be parsed because it is for a different network #26290

DrahtBot · 2023-03-15T10:17:19Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/27260.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	Sjors, rkrux, jonatack, l0rinc
Approach ACK	RandyMcMillan

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31974 (Drop testnet3 by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake · 2023-03-15T10:54:01Z

Please keep your changes, and "fixes" commits squashed. Looks like your changing the file perms on at least one file, is that intentional?

test/functional/rpc_invalid_address_message.py

portlandhodl · 2023-03-15T11:14:21Z

Please keep your changes, and "fixes" commits squashed. Looks like your changing the file perms on at least one file, is that intentional?

Will do. No it was not intentional but was a side effect of moving 3 files over scp to a different host. This has been resolved and commits squashed.

maflcko · 2023-05-18T14:19:53Z

Needs rebase on current master, if still relevant

portlandhodl · 2023-05-19T06:21:45Z

Needs rebase on current master, if still relevant

I believe this PR is still very relevant because it substantially improves the logic around address decoding and specifically the displaying of errors in the GUI and RPC. With that said I have rebased and this code is passing all tests other than Win64 native [vs2022] which seems to be failing for the majority of PRs that are on master due to warnings.

maflcko · 2023-05-19T06:25:04Z

Thanks, the reason I mentioned it was the silent merge conflict: key_io.cpp:152:48: error: ‘const class CChainParams’ has no member named ‘NetworkIDString’, which is now fixed

portlandhodl · 2023-05-19T06:30:10Z

Thanks, the reason I mentioned it was the silent merge conflict: key_io.cpp:152:48: error: ‘const class CChainParams’ has no member named ‘NetworkIDString’, which is now fixed

Is there a good way to detect these silent conflicts earlier? Usually I get notified via email when there are issues that need rebased; this is the first time one has happened without a notification.

Thanks

DrahtBot · 2026-01-08T08:37:21Z

🚧 At least one of the CI tasks failed.
_{Task macOS-cross to arm64: https://github.com/bitcoin/bitcoin/actions/runs/20810552653/job/59773530053}
_{LLM reason (✨ experimental): Build failed due to undefined symbol MAX_BASE58_CHARS in key_io.cpp (use of undeclared MAX_BASE58_CHARS).}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

portlandhodl · 2026-01-08T10:22:12Z

Quick note as of 6bcaa46. IIUC you rebased, and then dropped 055dbae Added chaintype user-facing string display, adjusting things where needed. But I guess you're still working on potentially refactoring 870c850 Base58 decoding logic + bech32 decoding network awareness and 2df89b6 Bech32 decoding logic based on earlier reviews?

ef5e517 I am using this as a setup commit for the more logic like sections that are handled in key_io.cpp

One of the hardest portions seems to be is that at some point this all converges into a case where you have to test that you don't have valid base58 or valid bech32 which inherently will have to run checks for both.

- Add common prefixes and values to chainparams will be used in error string emmited from key_io.cpp. - This approach is sane and stable as chains don't change and these constants have becoome socal community consensus.

Sjors · 2026-01-09T02:28:23Z

The "test max 6 ancestor commits" job fails at eacf78c General address decoding logic exe order.

This should fix it, by moving later changes into that commit: Sjors@aabb7df

Some of the commit messages have weird trailing characters (adasda, etc).

Noticed there's still a lot of bech32 related changes in c2c8454 Base58 decoding logic: despite the commit title, maybe split it further?

- Fixes the core of incorrect network bech32 being decoded as Base58 - Requires an upfront bech32 decode and base58 decode (performance traded for accuracy) adasda sdfds

- Add expexted prefixes to base58 decoding based on network - Rework the case of invalid base58 and invalid bech32

- Add network prefixes to the bech32 decoding layer

- completely refactor testing to support network dependant tests and fully validates that error messages are diplayed correctly. - JSON test vector storage with migrated vectors + comments asdas

portlandhodl · 2026-01-09T02:46:03Z

The "test max 6 ancestor commits" job fails at eacf78c General address decoding logic exe order.

This should fix it, by moving later changes into that commit: Sjors@aabb7df

Some of the commit messages have weird trailing characters (adasda, etc).

Noticed there's still a lot of bech32 related changes in c2c8454 Base58 decoding logic: despite the commit title, maybe split it further?

Thank you for the fixup! Applied

Edit- Per the messages, it was getting late and I was flogging the rebase -i and made a mistake

l0rinc · 2026-01-13T12:55:57Z

@portlandhodl, ping us when the PR is ready for review again.

DrahtBot added the RPC/REST/ZMQ label Mar 15, 2023

fanquake reviewed Mar 15, 2023

View reviewed changes

test/functional/rpc_invalid_address_message.py Show resolved Hide resolved

portlandhodl force-pushed the 26290 branch from 0b5cd1e to d9654cf Compare March 15, 2023 11:07

portlandhodl force-pushed the 26290 branch 7 times, most recently from c98f7cc to 911c8b6 Compare March 16, 2023 00:10

portlandhodl changed the title ~~rpc: enhanced error message for invalid network prefix during address parsing.~~ Enhanced error messages for invalid network prefix during address parsing. Mar 16, 2023

portlandhodl force-pushed the 26290 branch 6 times, most recently from 3c0fcda to aff7635 Compare March 19, 2023 08:50

DrahtBot added the CI failed label May 18, 2023

portlandhodl force-pushed the 26290 branch 2 times, most recently from ef46a30 to 5ff2490 Compare May 19, 2023 03:09

portlandhodl force-pushed the 26290 branch from 5ff2490 to 56908f1 Compare May 19, 2023 23:07

DrahtBot removed the CI failed label May 20, 2023

DrahtBot mentioned this pull request May 23, 2023

rpc: Fix invalid bech32 address handling #27727

Merged

portlandhodl force-pushed the 26290 branch 2 times, most recently from 73e3246 to 4232872 Compare January 8, 2026 08:36

DrahtBot added the CI failed label Jan 8, 2026

portlandhodl force-pushed the 26290 branch 7 times, most recently from 921a79c to a3e6139 Compare January 8, 2026 09:53

portlandhodl force-pushed the 26290 branch 3 times, most recently from 223a709 to 42bddb2 Compare January 8, 2026 10:41

portlandhodl marked this pull request as ready for review January 8, 2026 10:55

portlandhodl requested a review from Sjors January 8, 2026 10:56

Include Base58 encoded prefixes in chainparams

441ff6a

- Add common prefixes and values to chainparams will be used in error string emmited from key_io.cpp. - This approach is sane and stable as chains don't change and these constants have becoome socal community consensus.

portlandhodl force-pushed the 26290 branch 4 times, most recently from cd3cd31 to 6bc621e Compare January 8, 2026 18:21

portlandhodl added 4 commits January 9, 2026 02:42

General address decoding logic exe order

f35c8e7

- Fixes the core of incorrect network bech32 being decoded as Base58 - Requires an upfront bech32 decode and base58 decode (performance traded for accuracy) adasda sdfds

Base58 decoding logic

2edccf8

- Add expexted prefixes to base58 decoding based on network - Rework the case of invalid base58 and invalid bech32

Bech32 decoding logic

bb3c1a1

- Add network prefixes to the bech32 decoding layer

Introduce muti network testing for rpc_validateaddress.py

20138f8

- completely refactor testing to support network dependant tests and fully validates that error messages are diplayed correctly. - JSON test vector storage with migrated vectors + comments asdas

portlandhodl force-pushed the 26290 branch from 6bc621e to 20138f8 Compare January 9, 2026 02:44

Enhanced error messages for invalid network prefix during address parsing. #27260

Are you sure you want to change the base?

Enhanced error messages for invalid network prefix during address parsing. #27260

Conversation

portlandhodl commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Improve Address Decoding Error Messages

Issue

Solution

Implementation Approach

Changes Made

1. Add Base58 Encoded Prefixes to ChainParams (src/kernel/chainparams.cpp, src/kernel/chainparams.h)

2. Refactor Address Decoding Logic (src/key_io.cpp)

3. Improved Error Messages

4. Test Updates

Consistency Improvements

Reference

Uh oh!

DrahtBot commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

fanquake commented Mar 15, 2023

Uh oh!

Uh oh!

portlandhodl commented Mar 15, 2023

Uh oh!

maflcko commented May 18, 2023

Uh oh!

portlandhodl commented May 19, 2023

Uh oh!

maflcko commented May 19, 2023

Uh oh!

portlandhodl commented May 19, 2023

Uh oh!

DrahtBot commented Jan 8, 2026

Uh oh!

portlandhodl commented Jan 8, 2026

Uh oh!

Sjors commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

portlandhodl commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

l0rinc commented Jan 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

portlandhodl commented Mar 15, 2023 •

edited

Loading

1. Add Base58 Encoded Prefixes to ChainParams (`src/kernel/chainparams.cpp`, `src/kernel/chainparams.h`)

2. Refactor Address Decoding Logic (`src/key_io.cpp`)

DrahtBot commented Mar 15, 2023 •

edited

Loading

Sjors commented Jan 9, 2026 •

edited

Loading

portlandhodl commented Jan 9, 2026 •

edited

Loading