Protocol Change Proposal: Fix Bech32 Address Support

[adamkrellenstein]

Fix Bech32 Address Support

Motivation

It has been 8 years since Bech32 addresses were introduced and became the standard for most wallets.

Bech32 address support was added to Counterparty in 2018 with the following code to get the address from the script_pubkey:

bech32 = bitcoinlib.bech32.CBech32Data.from_bytes(0, script_pubkey[2:22])

This line works well for P2WPKH addresses but silently outputs the wrong values for P2WSH and P2TR addresses which are converted to P2WPKH addresses (20 bytes of script instead of 32). In 2024 during parsing optimization, this method was rewritten in Rust and the bug was intentionally reproduced to maintain backwards-compatibility. The proposal is to fix this bug, which requires a protocol change.

Current Parser Support

Address Type	Support Status
P2PK	Supported
P2PKH	Supported
P2MS	Supported
P2SH	Supported
P2WPKH	Supported
P2WSH	Supported with bug: scripts truncated to 22 bytes as for a P2WPKH address
P2TR	Supported with bug: scripts truncated to 22 bytes as for a P2WPKH address

Code Changes

To add support for a type, we must do it in 3 places:

• When we parse the outputs to determine the destination (function parse_vout() in Rust)
• When we parse the inputs to determine the source (function get_transaction_sources() in Python)
• In the transaction composer (function address_to_script_pub_key() in Python)

For the first two points, it's just a matter of fixing the script_to_address() function.

In the case of Composer it is just a matter of using the correct class from the bitcoin-utils library.

The real problem is managing the numerous balances, orders, dispensers, etc. registered in the database with incorrect addresses.

Methodology

The cleanest method is to do a complete database reparse to fix all incorrect addresses. This means that all checkpoints need to be updated and we cannot use them to verify that the bug fix has no side effects elsewhere in the code.

To solve this problem and be able to verify that nothing has changed except the addresses, a new hash will be temporarily introduced. Currently, the ledger_hash is calculated by concatenating the block_index, address, asset, and quantity for each row added to the debits and credits tables.

The idea is to add a new hash using only the first 36 characters (4 for the prefix, 32 for the first 20 encoded bytes) of the address instead of the complete address:

1. Step 1: Add a new migration_hash and completely reparse the database to calculate it for all blocks
2. Step 2: Fix script_to_address() function
3. Step 3: Reparse to calculate the new ledger_hash while ensuring the migration_hash remains identical
4. Step 4: Use the migration_hash as the new ledger_hash

API Changes

No change

Database Changes

No change

[ouziel-slama]

done here #3052

[jdogresorg]

I disagree with your assertion that the "cleanest" method is to break 10+ years of hashes which were used to validate that data has been parsed correctly.

IMO The "cleanest" way to fix this would be to simply fix the issue at a given block_index via a protocol change, as has been the case with all fixes to CP since the beginning of time. At no point has anyone gone back through and recalculated all the hashes, destroying the ability for anyone to compare hashes that have been rock solid for 10 years.

IMO, this is yet another oversight on your part, and hurts CP long-term... your essentially going back to 2014, forking, and calling your change "Counterparty".... Yet another change which you feel is best, so it is forced on the community with no true discussion, and people are left scratching their heads as to why hashes which have been used to verify that data was parsed correctly for over 10 years have suddenly changed.

At this point you might was well go with the "cleanest" way to import data and simply import all txs via a CSV instead of parsing historical txs from the blockchain, as there is no longer a way to validate that the data has been parsed correctly.

The Bitcoin mantra is "Don't Trust, Verify".... but now it seems that with this change the CP mantra is "Don't verify, Trust".

Edit: Also seems that this change has resulted in new asset owners for certain tokens... even worse than changing hashes, we now are changing asset owners with this change.

bc1qpx40lk8luqqpqjjxf9rqqqgpqyqysqzg3hra0j = https://tokenscan.io/asset/PEPOLGA (v10.10.1)

bc1qpx40lk8luqqpqjjxf9rqqqgpqyqysqzgqqq0lkcqgvqpqzcvpcxqlnp0ns = https://horizon.market/explorer/assets/PEPOLGA (v11.0.0)

[Jpja]

Version 11, which has the proposed retroactive fix implemented, diverges at block block 334,138 – in December 2014.

Version 10, which is still run by the majority, matches the hashes in check.py all the way up to the very last checkpoint.

I consider 10+ years of matching hashes a very valuable property. Version 10 matches v9 up until it forked, so did v9 with v8's hashes and so on. Forks always took place from an activation block, never retroactively!

I understand that this proposal is well intended, and that it will unlock some frozen funds. But have you analyzed exactly how many balances (and how much value) that will be retrieved? I suspect it's negligible.

Even worse, what if someone camouflaged a real P2WPKH address as a (fake) P2WSH address at some point?

Example
P2WSH: bc1qcpnews0493hkswn4vw8d5vaq6e6g59n5tanx7untv4e8qctjw3usdlw5kh / **c0679741f52c6f683a75638eda33a0d6748a1674**5f666f726b65727061727479
P2WPKH: bc1qcpnews0493hkswn4vw8d5vaq6e6g59n5qyvjpq / c0679741f52c6f683a75638eda33a0d6748a1674.

In v10 and earlier a send to P2WSH would debit the P2WPKH. These tokens could have be traded with others and spread around the ecosystem by now, In contrast, on v11 the tokens would get debited to the P2WSH address and all txs directly and indirectly from P2WPKH will be invalid.

Furthermore, v11 opens up for potential new bugs all the way back to 2014. At block 334138, where v10 and v11 diverge, a token is sent to a weird looking address: 2_1ETcsB7sA14VZ1kyFx5R3AHqsu9naTKWwu_1L5LrXY7P8P9JYmd59o3bQq6zBvnJBMPmB_2. Both v10 and v11 seem to parse this identically, so why the block mismatch? And why a mismatch when that address format is not mentioned in the proposal?

Regarding PEPOLGA, as mentioned by @jdogresorg, it is actually from my OLGA Stamp Minter. It's been used by several artists and all their assets (100+?) will get ownership changed in retrospect. Not nice optics imo :/

Why not just fix Bech32 Address Support starting at a set future block?

[adamkrellenstein]

What a bunch of nonsense. There were eight years of no checkpoints at all, and a dozen consensus breaks during that time #1310 The historical consensus hashes are a mere sanity check, and they're verified during the v11 migration to make sure that no history was changed. Read the actual code: #3052