kernel: Remove key module from kernel library

[sedited]

The key module's functionality is not used by the kernel library, but currently kernel users are still required to initialize the key module's secp256k1_context_sign global as part of the kernel::Context through ECC_Start. So move the ECC_Start call to the NodeContext ctor instead to completely remove the key module from the kernel library.

The gui tests currently keep multiple NodeContext objects in memory, so call ECC_Stop manually to avoid triggering an assertion on ECC_Start.

---

This PR is part of the libbitcoinkernel project. It removes a module from the kernel library.

[DrahtBot]

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky, theuni, achow101
Stale ACK	maflcko, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

[maflcko]

The gui tests currently keep multiple NodeContext objects in memory, so relax the secp256k1_context_sign check in ECC_Start to instead return early if it has already been initialized.

An alternative would be to call ECC_Stop in the gui tests instead of modifying key code?

[sedited]

Updated 67058d4a82e222f770ede96dc948f758b0a8386c -> 2cb38d93a8fea16bf84b072a90ac023ef345134d (kernelRmKey_0 -> kernelRmKey_1, compare)

• Implemented @maflcko's suggestion, calling ECC_Stop instead of relaxing the ECC_Start check.

[maflcko]

lgtm ACK 2cb38d93a8fea16bf84b072a90ac023ef345134d 🍂

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK 2cb38d93a8fea16bf84b072a90ac023ef345134d 🍂
3Z6w9mvDbuK4T+f2+g2HGbUkY+ETKts/CGAUhdhsu/MjBFRcdyVX4GOLme1SMwENKvYIENkhNym7luWlGcHRAw==

[sedited]

Updated 2cb38d93a8fea16bf84b072a90ac023ef345134d -> a885a166cec6d84d08600f12b25d912bd28af80e (kernelRmKey_1 -> kernelRmKey_2, compare)

• Addressed @maflcko's comment, re-added explicit context destructor.
• Addressed @maflcko's comment, removing dangling include for key.h.

[maflcko]

lgtm ACK a885a166cec6d84d08600f12b25d912bd28af80e 🔀

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK a885a166cec6d84d08600f12b25d912bd28af80e 🔀
MgtL4Gf030oUpUZQHa33bgM/KAIA6X7ogn4k7avoYVc87vxEv1qrxN9wxdvRTYiOZ1Hd8faqpxGbrRFpJIA9DQ==

[hebasto]

Concept ACK.

[theuni]

@TheCharlatan Mind adding a commit to fixup the includes as per this conversation?

[sedited]

Thank you for the review @ryanofsky!

a885a166cec6d84d08600f12b25d912bd28af80e -> 7dc7938731233f5f3299618e1e07ff92782a7cfd (kernelRmKey_2 -> kernelRmKey_3, compare)

• Addressed @ryanofsky's comment, cherry-picked the mentioned commits. Also added another commit for removing the now unused ECC_Start and ECC_Stop from the header.
• Addressed @theuni's comment, cleaning up some of the includes.

[sedited]

Rebased 7dc7938731233f5f3299618e1e07ff92782a7cfd -> 65a205166b3c7d082a7528ea5036dff551d7eb6e (kernelRmKey_3 -> kernelRmKey_4, compare)

[sedited]

Updated 65a205166b3c7d082a7528ea5036dff551d7eb6e -> 96378fe734e5fb6167eb20036d7170572a647edb (kernelRmKey_4 -> kernelRmKey_5, compare)

• Addressed @ryanofsky's comment, moving comments and improving description of ECC_Context.

[achow101]

ACK 96378fe734e5fb6167eb20036d7170572a647edb

[hebasto]

Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/202.