Bech32 LocateErrors "Bech32 string too short" case #28160

Summary

Add a "Bech32 string too short" case to Bech32.cpp LocateErrors to reduce ambiguity and collisions with "Invalid separator position" case.

Problem

Error representation when calling LocateErrors within src/test/bech32_tests.cpp is ambiguous and incorrect in some cases because of a missing bech32 string to short case. Not having a too short case causes a fall though into the conditions of "Invalid separator position" || "Missing separator"

Solution

Simply add a check for a minimum Bech32 string size after or during the check for the maximum size which is already implemented.

Cascading effects of this would be seen in DecodeDestination if https://github.com/bitcoin/bitcoin/blob/32c15237b656209b932c5d6d2e20736c0e3d5a34/src/key_io.cpp#L85-L86 was patched to not consider bech32 invalid then attempt to Base58Decode it becuase of the incorrect HRP for the current chain.

Referenced in #26290

PR #27260

Alternatives

You could case this into other logic on a case by case basis in code. Example make a speical case inside DecodeDestination to display the correct error message to the user by if LocateErrors returns 'Invalid "separator position" || "Missing separator" then you check to see if the string is to short.

Additional Context

As an aside there is already a check for string length greater than 90 (MAX LEN)

in src/test/bech32_tests.cpp both ["10a06t8","1qzzfhee"] produce {"Invalid separator position", {0}} The former is non conformant to BIP173 due not even having enough chars to represent a BASE32 string and the second genuinely has a wrong separator position.

Adding this case would make DecodeDestination more accurate for anyone who wants to to implement bech32.{cpp,h}

A comparison of this PRs error vs the current 25.99 error for Bech32 string that does not meet the minimum length requirement.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

The new error_locations looks worse/broken...

First, Thanks for your feedback and time to look at this.

Though the error locations for the too short condition are not aesthetically pleasing at longer lengths; it does look nicer at shorter lengths of a too short bech32 string. Adding this condition does increase the accuracy of LocateErrors by covering a case where when executed currently displays an incorrect error. Ideas and suggestions are very welcome.

Example where someone enters only the hrp (3 chars)

Rationale

The error_locations for this PR took inspiration from the current error_locations for the too long condition of LocateErrors that when triggered will display the positions of any additional characters past the 90th. My thought was to show all character positions that violate the too short condition. I also chose this method over some of my other alternatives because though uglier at longer lengths it is accurate and there is a limit to the total number of positions displayed at 7. per BIP173.

Expected outputs for this PR error locations

• Bc1 [0,1,2]
• Bc1l [0,1,2,3]
• Bc1lu [0,1,2,3,4]
• Bc1luke [0,1,2,3,4,5,6,7]

Below is an example of comparing a string that is too short and on where the string to too long.

Context

https://github.com/bitcoin/bitcoin/blob/d096743150fd35578b7ed71ef6bced2341927d43/src/bech32.cpp#L399

Alternatives

• Don't display the error message for a too short condition - I didn't consider this an option because of the fact that the current implementation of locate errors gives an incorrect error ("Invalid separator position" || "Missing separator")

• Don't display any error_locations position data at all - This to me would be valid because the error itself conveys enough information for the user to likely fix the problem. This solution is inconsistent with the current implementation. Applying that logic to too long conditions would mean the removal of error_locations positions for the too long condition since that is also quite obvious what happened.

• Display the start and end positions in the array. This was another solution I had thought about since it would limit the fix number of characters in the array to 2. This didn't seem consistent with the current implementation of LoateErrors and very deliberately defeats the purpose of the locate_errors array of incorrect characters.

• Display the end position in the array. Mostly the same reason as above.

Combining into 27620