Concept ACK, I agree this shouldn't cause a crash.

The root cause of the problem this is fixing is that the wallet does not contain the upgraded key metadata because it is encrypted and has not been unlocked yet, and a transaction was found that involved inactive HD chains. So this segfault can be triggered with a non-corrupted wallet. I don't think it is necessarily correct to log an error and move on as doing so could result in missing funds.

To avoid the crash, I think access to meta.key_origin.path should be guarded by both the length check and meta.has_key_origin. Then if has_key_origin == false, meta.hdKeypath can be used to figure out what to derive. This would avoid the segfault while not losing any functionality.

when I encountered this, this was the state of meta,

(gdb)
p meta
$1 = {static VERSION_BASIC = 1, static VERSION_WITH_HDDATA = 10, static VERSION_WITH_KEY_ORIGIN = 12, 
  static CURRENT_VERSION = 12, nVersion = 10, nCreateTime = 1509688921, hdKeypath = "m/0'/0'/0'", 
  hd_seed_id = {<uint160> = {<base_blob<160>> = {static WIDTH = 20, 
        m_data = "4\264Y\351....", <incomplete sequence \313>}, <No data fields>}, <No data fields>}, key_origin = {fingerprint = "\000\000\000", path = std::vector of length 0, capacity 0}, has_key_origin = false}
(gdb)
                                                                             

when I encountered this, this was the state of meta,

This matches what I expect. The meta is not upgraded (nVersion is 10, not 12, and has_key_origin is false).

Please squash your commits.

I will implement the derivation from the keypath string in a followup PR as that is a bit more complicated to deal with.

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #23304 (wallet: Derive inactive HD chains in addtional places by achow101)

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.

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

@rooprob Are you planning on continuing to work on this PR? If not, I can take it over in #23304

@rooprob Are you planning on continuing to work on this PR? If not, I can take it over in #23304

I think it's better to follow up in your PR. I shall withdraw this PR.

Thanks for picking this up in #23304