Repository navigation
mhda 1.2.0: derivation index range, stricter grammar and validation - #2
Merged
Merged
Conversation
The CIP-11 charge and the CIP-1852 role accept any level index, but charge_type was std::uint8_t, so the parsed value was truncated to a byte: m/1852'/1815'/0'/256/0 parsed as role 0, re-serialised as m/1852'/1815'/0'/0/0 and returned role 0 in levels(). Two URNs named the same key. charge_type is now std::uint32_t and keeps the full value in charge(), levels() and str(). Mirrors go-mhda. This changes the ABI of derivation_path; CHANGELOG notes it under 1.2.0.
The purpose, the fixed coin of cip11/cip1852/zip32 and the 0|1 charge of bip32 and the BIP-44 family are literals in the go-mhda regexes, so Go refuses a leading zero there (m/044'/60'/0'/0/0, m/44'/60'/0'/00/0, m/1852'/01815'/0'/0/0). The port compared the parsed values only and accepted them: a URN Go refuses parsed here. Each level now records whether its digits carry a leading zero, and the fixed levels refuse it with invalid_derivation_path. Variable levels still accept leading zeros and drop them in str(), as in Go. SPEC.md writes down the rule (identical text in go-mhda).
A BIP-32 child number keeps the hardened flag in its top bit: n' is child number 2^31+n, so a level index must stay below 2^31. parse_segment accepted any 32-bit value, and a consumer computing hardened ? 0x80000000|index : index got an alias: m/44'/60'/2147483648'/0/0 derived the key of m/44'/60'/0'/0/0, and an unhardened .../0/2147483648 asked for a hardened child number through the public-key formula. parse_uint32_dec becomes parse_uint31_dec, used by every level of every derivation type, so parse_urn, parse_nss, derivation_path::parse and validate_derivation_path refuse such a path with invalid_derivation_path. The ct component is not a path level and keeps its 32-bit range. Mirrors go-mhda. The boundary test and the fuzz seed that pinned 4294967295 now sit at the new bound, and the derivation-path fuzz checks that no accepted level exceeds 2^31-1. tests/data/dp_conformance.txt is the table shared verbatim with go-mhda (testdata/dp_conformance.txt): every row runs standalone, through validate_derivation_path and inside a URN, so a path one implementation refuses, the other refuses too. Version 1.2.0. SPEC.md (same text as go-mhda), CHANGELOG.md and README describe the rule and the URNs that no longer parse.
The named constants (network_type::bitcoin, algorithm::secp256k1, format::hex, derivation_type::root, ...) were namespace-scope objects defined in the library's .cpp files, built at start-up in an unspecified order relative to a consumer's globals, and the lookup tables copied them on first use. A consumer global that ran first saw empty values and left the tables wrong for the whole process: parse_urn_strict then refused every network as unknown, even from main(). A global destructor that parsed at exit read tables already destroyed and crashed. The constants are now inline variables defined in the public headers: every translation unit that names one includes the definition, so it is initialised before any global defined after the include. The lookup tables (network index, algorithm/format/derivation registries, component set, compatibility matrix) are allocated once and never destroyed. tests/test_static_init.cpp holds a global that parses during static initialisation and one that parses in its destructor; before the change the suite crashed.
The network type, the chain id and the derivation type are written verbatim into every URN and chain key, but nothing checked them when they were set programmatically. A chain id such as 1:dt:bip44:dp:m/44'/60'/0'/0/666, set through the chain constructor or address::get_chain().set_chain_id on a root address, produced a URN that re-parsed as a bip44 path. The chain constructor, set_network and set_chain_id now require a registered network type and a non-empty chain id of printable ASCII without ':', '?' or '#' (ASCII-trimmed, the rule of the free-form setters, now shared through detail::validate_free_form_value) and throw parse_error otherwise, leaving the chain unchanged. The derivation_path constructor, from_levels and set_type throw parse_error(invalid_derivation_type) on an unregistered type. Mirrors go-mhda.
set_derivation_path returned early for a root address, so a dp with no dt (which parses as root) or with dt:root was dropped silently: urn:mhda:nt:evm:ci:1:dp:m/44'/60'/0'/0/0 parsed as urn:mhda:nt:evm:ci:1, naming the root key instead of the path it spells out. derivation_path::parse_path already refused a non-empty path for root; the address setter now does the same and throws invalid_derivation_path. An empty dp under root stays a no-op. Mirrors go-mhda. tests/data/urn_conformance.txt is the URN-level table shared verbatim with go-mhda (testdata/urn_conformance.txt), run through parse_urn / parse_nss; this change adds its first rows.
parse_nss_map skipped an unknown token on its own, so the token after it was read as a key: a value could become a key and a known key could become a value. A key with the wrong case was skipped with it, dropping the component silently. ...:ci:1:DT:bip44:DP:m/44'/60'/0'/0/5 parsed as the root address urn:mhda:nt:evm:ci:1, and ...:ci:1:ext:wi:dt:bip44:... took dt as the wallet id. The NSS is now split into pairs: an odd number of tokens is refused, an unknown component is skipped together with its value, and a key that differs from a known key only by case throws invalid_nss. Keys must be non-empty printable ASCII, like values. An empty NSS still has no components. chain::from_key keeps reporting a dangling token as invalid_chain_key. Mirrors go-mhda.
parse_urn strips the RFC 8141 r/q/f components at the first '?' or '#', but parse_nss, chain::from_nss and chain::from_key took the NSS as given and kept the byte in a value, so the URN emitted from such an address was truncated on the next parse. The chain-id check of the previous change caught it in ci with the wrong code; in other values (nt:evm:ci:1:x#y:z) it passed. parse_nss_map now refuses either byte with invalid_nss, as go-mhda does. The NSS fuzz test only exercised str() and nss(); it now re-parses the emitted URN and compares, with the truncating inputs as seeds.
BIP-32 serialises a key's depth in one byte, so a path deeper than 255 levels names a key no wallet can represent; validate_slip10 accepted any depth, and a boundary test pinned a 256-level path as valid. A slip10 path with more than 255 levels now throws invalid_derivation_path, as in go-mhda; the boundary test and the shared derivation-path table cover 255 and 256.
SPEC.md promised that a long form round-trips to itself, but an explicit dt:root is folded away: ...:ci:1:dt:root serialises as ...:ci:1. That is the intended behaviour (root is the default and has no path, so a root address keeps one canonical form and one hash, and consumers rely on it); the spec now says so, and the Algorand and TON notes no longer call dt:root the canonical form. zip32 parses, but no network registers it, so parse_urn_strict refuses it everywhere; the spec lists that as a known limitation. Tests pin both: dt:root rows in the shared URN table and a strict-refusal check over every network. Same text as go-mhda.
SHA-1 and SHA-256 are hand-written here, and the only reference test hashed one 26-byte string, which never reaches the two-block padding path. Add reference digests for str() of 55, 56, 63, 64, 119, 120, 128 and 1000 bytes (the NSS is 9 bytes shorter, so it covers 55 too), computed with Python's hashlib; go-mhda pins the same values. An off-by-one in the padding length now fails the test at 55 and 119 bytes.
CI built and tested the Release/Debug matrix only, while the README promises a clean run under ASan/UBSan and under a strict warning set. Add a job that builds with -fsanitize=address,undefined and runs the suite with halt_on_error, and a job that builds with the README's warning set as errors. The warnings job is pinned to ubuntu-22.04 (GCC 11, the compiler the README lists as verified); the same flags are also clean with GCC 12 and Clang 14. The workflow runs with read-only permissions. README test counts and the 1.2.0 CHANGELOG test notes are brought up to date (152 cases).
from_levels kept the levels it was given while str() printed the type's template: a BIP-44 path built from 49'/60'/0/0/0 printed m/44'/60'/0'/0/0 but returned purpose 49' and an unhardened account from levels(), which is what a wallet derives from. Too few levels (none for SLIP-10) or an index of 2^31 gave a URN that does not parse, and the five-argument constructor accepted a charge of 7 or an account of 2^31. Both constructors now return the path parse() gives back for their own str() and throw parse_error(invalid_derivation_path) unless it has exactly the requested levels. The boundary tests that pinned the old results for an empty SLIP-10 and a two-level BIP-44 path now expect the refusal, and the fuzz tests check that a re-parsed path has the same levels. Mirrors go-mhda.
set_type and address::set_derivation_type changed the type and kept the old path: a bip44 address switched to zip32 serialised as dt:zip32:dp:m/32'/133'/3'/9, a valid URN naming a key nobody gave, and a SLIP-10 path parsed after it kept the BIP-44 coin, account and charge. set_derivation_type followed by a failing set_derivation_path left that mixed address behind, and a bad_alloc inside parse_path left str() and levels() disagreeing. A new type now clears the path; until one is set, str() of the path is empty, the URN carries dt without dp (so it does not parse) and validate / marshal_text throw invalid_derivation_path. parse_path parses into a fresh path and move-assigns it, so it replaces the old path only on success (strong guarantee). address::set_derivation(dt, dp) sets both at once and changes nothing on error; the URN parser uses it. Mirrors go-mhda.
validate checked the algorithm, the format and the derivation type each against the network on its own. It accepted SLIP-10 ed25519 paths with soft levels (ed25519 has no soft derivation), bip44 with ed25519 on XRPL, NEAR and Aptos, Sui bip54 with ed25519 or secp256r1 (the purpose encodes the scheme), a Bitcoin bip84 path with af:p2pkh, and Cosmos bip44 with coin 118', which is the cip11 path under a second name. The compatibility matrix now binds derivation types to their curve on Sui, Aptos and NEAR and Bitcoin purposes to their scripts, and validate also refuses, with incompatible: an ed25519 path with an unhardened level, except cip1852 (BIP32-Ed25519); a derivation type with another curve than its own; an explicit Bitcoin format that does not match the purpose; and Cosmos bip44 with coin 118'. A Bitcoin URN without af stays valid. Mirrors go-mhda. The shared URN table gains strict rows (the runner learns the strict kind and the incompatible code). Tests that pinned now-refused pairs move to matching purposes, and the XRPL ed25519 example becomes the root form its family-seed keys have.
Embedded with add_subdirectory or FetchContent, mhda forced CMAKE_BUILD_TYPE=Release into the cache of a parent that had none, so the parent's own code was built with -O3 -DNDEBUG; it also built its tests and examples into the parent and added its install rules there. These now apply only to a top-level build, and MHDA_BUILD_TESTS, MHDA_BUILD_EXAMPLES and the new MHDA_INSTALL default to it. BUILD_SHARED_LIBS=ON gave a libmhda.so without any mhda symbol (hidden visibility, no export macros); a shared build now exports its symbols (WINDOWS_EXPORT_ALL_SYMBOLS for a DLL) and passes the suite, a static one keeps them hidden. mhda::mhda requires cxx_std_17 publicly, so a C++14 consumer is raised to C++17 instead of failing on std::optional, and the install include path follows CMAKE_INSTALL_INCLUDEDIR. tests/cmake holds a find_package consumer (asking for C++14) and an add_subdirectory consumer that fails if mhda touches the parent's build type or targets; CI builds both and a shared library.
<windows.h> (minwindef.h) defines near as an empty macro, so constexpr coin_type near = 397 in coin_type.hpp became a syntax error in any translation unit that included <windows.h> first; the wallet builds for Windows. coin_type.hpp now sets the macro aside with push_macro / pop_macro around the coin list, and adds coins::near_protocol, which stays usable while the macro is in effect. A test translation unit defines near and far the way <windows.h> does.
parse_error::what() copied the caller's input verbatim into the message,
so a newline or an ANSI escape in a client-supplied value
(set_wallet_id("x\n[INFO] ok")) forged or coloured lines in any log that
recorded the exception. Go escapes the same messages with %q.
detail::quote escapes '"', '\', \n, \r, \t and every other control or
non-ASCII byte (\xNN), and every message that quotes input uses it:
setter values, derivation types and paths, chain keys, coin types and
NSS keys. Messages made only of validated values are unchanged.
parse_nss_map trimmed every value, so urn:mhda:nt:evm:ci: 1 :dt: bip44 :... parsed like the same URN without the spaces, although SPEC section 6.1 says values contain no whitespace and only the whole URN is trimmed. parse_nss_map no longer trims; whitespace in a key or value fails the charset check with invalid_nss. parse_urn trims the URN and the whitespace left before a stripped r/q/f component (ci:0 #frag still parses); parse_nss, chain::from_nss and chain::from_key trim the NSS they are given as a whole. Mirrors go-mhda.
Keys and values accepted any printable ASCII, so '"', '<', '>', '\',
'^', '`', '{', '|', '}', '[', ']' and a raw '%' passed and were emitted in
URNs that RFC 8141 does not allow, although SPEC section 1.5 requires the NSS
production (RFC 3986 pchar and "/").
detail::nss_byte now admits letters, digits and -._~!$&'()*+,;=@/ -
pchar and "/" without ':' (the separator) and '%' (percent-encoding is
not supported, and %41 would be a second spelling of "A"). parse_nss_map
uses it for keys and values (invalid_nss), validate_free_form_value for
ap/as/wt/wi and the chain id (invalid_value). The shared URN table is
again identical to go-mhda's. Mirrors go-mhda.
ci:1, ci:0x1 and ci:01 are three chain keys and hashes for one chain. MHDA keeps the chain id opaque; SPEC.md now says so and asks a producer to keep to one spelling (the EIP-155 decimal form is the usual one), and carries go-mhda's codec note, as in go-mhda. The 1.2.0 changelog had grown entry by entry; it is regrouped into Changed (inputs refused now), Fixed, Documentation and Tests, and the README status and test counts (160 cases) are brought up to date.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Version 1.2.0, a behavioral mirror of go-mhda 1.2.0. URNs and values accepted by 1.1 are refused now; the full list is in CHANGELOG.md. The URN grammar stays bit-for-bit compatible with the Go reference.
What changes
invalid_derivation_path); it collided with the hardened bit (m/44'/60'/2147483648'/0/0derived the key ofm/44'/60'/0'/0/0). Fixed levels (purpose, fixed coin, 0|1 charge) are spelled exactly, as in Go.charge_typeis 32 bits wide (wasstd::uint8_t): role 256 named the role-0 key. ABI change.key:valuepairs, mis-cased keys refused;dpwithoutdtor under root refused;?/#refused in an NSS parsed on its own; no whitespace inside the NSS; RFC 3986 byte set; SLIP-10 at most 255 levels.parse_urn_strict/validate: hardened-only ed25519 (exceptcip1852), curve per derivation type on Sui, Aptos and NEAR, Bitcoin format matching the purpose, Cosmosbip44/118' refused forcip11.chainconstructor and setters, thederivation_pathconstructors andset_typethrow on values that would corrupt or misname a URN; constructed paths equal the parsed ones, solevels()andstr()agree.parse_pathreplaces the path with the strong guarantee; a new derivation type clears the path;address::set_derivation(dt, dp)sets both atomically.what()escapes the input it quotes; the headers compile under the<windows.h>nearmacro (newcoins::near_protocol).add_subdirectory/ FetchContent, mhda no longer forces the parent's build type nor adds its tests, examples or install rules (MHDA_INSTALLis new);BUILD_SHARED_LIBS=ONworks;cxx_std_17is a public requirement; the install include path followsCMAKE_INSTALL_INCLUDEDIR.-Werror(GCC 11), shared-library and packaging (find_package/add_subdirectoryconsumers) jobs.Testing
-fsanitize=address,undefined,leak, under-Werrorwith the README's warning set (GCC 11, GCC 12, Clang 14), and as a shared library.tests/cmakeconsumers build and run: afind_packageproject asking for C++14 and anadd_subdirectoryproject that fails if mhda touches its build type or targets.tests/data/dp_conformance.txtandtests/data/urn_conformance.txtare byte-identical to go-mhda's; a differential run against go-mhda 1.2.0 gave byte-identical output for 500 000 mutated URNs (lenient, strict, NSS, chain key) and 600 000 derivation paths.