-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Schnorr sign-to-contract and anti-exfil #1140
base: master
Are you sure you want to change the base?
Changes from all commits
26dafba
480e909
9ab5eb1
1b8b680
ab2c87c
49c3137
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
#ifndef SECP256K1_SCHNORRSIG_H | ||
#define SECP256K1_SCHNORRSIG_H | ||
|
||
#include <stdint.h> | ||
|
||
#include "secp256k1.h" | ||
#include "secp256k1_extrakeys.h" | ||
|
||
|
@@ -13,6 +15,63 @@ extern "C" { | |
* (https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki). | ||
*/ | ||
|
||
/** Data structure that holds a sign-to-contract ("s2c") opening information. | ||
* Sign-to-contract allows a signer to commit to some data as part of a signature. It | ||
* can be used as an Out-argument in certain signing functions. | ||
* | ||
* This structure is not opaque, but it is strongly discouraged to read or write to | ||
* it directly. | ||
* | ||
* The exact representation of data inside is implementation defined and not | ||
* guaranteed to be portable between different platforms or versions. It can | ||
* be safely copied/moved. | ||
*/ | ||
typedef struct { | ||
/* magic is set during initialization */ | ||
uint64_t magic; | ||
/* Public nonce before applying the sign-to-contract commitment */ | ||
secp256k1_pubkey original_pubnonce; | ||
/* Byte indicating if signing algorithm negated the nonce. Alternatively when | ||
* verifying we could compute the EC commitment of original_pubnonce and the | ||
* data and negate if this would not be a valid nonce. But this would prevent | ||
* batch verification of sign-to-contract commitments. */ | ||
int nonce_is_negated; | ||
} secp256k1_schnorrsig_s2c_opening; | ||
|
||
/** The signer commitment in the anti-exfil protocol is the original public nonce. */ | ||
typedef secp256k1_pubkey secp256k1_schnorrsig_anti_exfil_signer_commitment; | ||
|
||
/** Parse a sign-to-contract opening. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to add a succinct description of the actual protocol to this header, or a reference to it? The idea is that this is essentially defining a new cryptographic protocol that isn't formally specified anywhere. If so, it'd be good to have the specification right here, so someone doesn't need to go read through the implementation in order to figure out what the exact protocol is. That makes it easier to separately review the protocol itself, and compare the implementation to that protocol. As an example, see https://github.com/bitcoin-core/secp256k1/pull/1129/files#diff-d3089adfea8d8bbc31e130bb67389ab9ef45ea4abf4a79eceec63a037359f39dR10-R44 for example, which does so for ElligatorSwift. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a specification in the header (in a new commit for easier review, will squash in the end). The spec is mostly a copy/paste from A very similar spec was present in @jonasnick's original PR: |
||
* | ||
* Returns: 1 if the opening was fully valid. | ||
* 0 if the opening could not be parsed or is invalid. | ||
* Args: ctx: a secp256k1 context object. | ||
* Out: opening: pointer to an opening object. If 1 is returned, it is set to a | ||
* parsed version of input. If not, its value is undefined. | ||
* In: input33: pointer to 33-byte array with a serialized opening | ||
* | ||
*/ | ||
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_s2c_opening_parse( | ||
const secp256k1_context* ctx, | ||
secp256k1_schnorrsig_s2c_opening* opening, | ||
const unsigned char *input33 | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); | ||
|
||
/** Serialize a sign-to-contract opening into a byte sequence. | ||
* | ||
* Returns: 1 if the opening was successfully serialized. | ||
* 0 if the opening was not initializaed. | ||
* Args: ctx: a secp256k1 context object. | ||
* Out: output33: pointer to a 33-byte array to place the serialized opening | ||
* in. | ||
* In: opening: a pointer to an initialized `secp256k1_schnorrsig_s2c_opening`. | ||
*/ | ||
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_s2c_opening_serialize( | ||
const secp256k1_context* ctx, | ||
unsigned char *output33, | ||
const secp256k1_schnorrsig_s2c_opening* opening | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); | ||
|
||
/** A pointer to a function to deterministically generate a nonce. | ||
* | ||
* Same as secp256k1_nonce function with the exception of accepting an | ||
|
@@ -63,35 +122,66 @@ typedef int (*secp256k1_nonce_function_hardened)( | |
*/ | ||
SECP256K1_API const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; | ||
|
||
/** First version of the extraparams struct. See `secp256k1_schnorrsig_extraparams` for the | ||
latest version and its documentation. | ||
*/ | ||
typedef struct { | ||
unsigned char magic[4]; | ||
secp256k1_nonce_function_hardened noncefp; | ||
void *ndata; | ||
} secp256k1_schnorrsig_extraparams_v0; | ||
|
||
/** Data structure that contains additional arguments for schnorrsig_sign_custom. | ||
* | ||
* A schnorrsig_extraparams structure object can be initialized correctly by | ||
* setting it to SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT. | ||
* | ||
* Members: | ||
* magic: set to SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC at initialization | ||
* and has no other function than making sure the object is | ||
* initialized. | ||
* noncefp: pointer to a nonce generation function. If NULL, | ||
* secp256k1_nonce_function_bip340 is used | ||
* ndata: pointer to arbitrary data used by the nonce generation function | ||
* (can be NULL). If it is non-NULL and | ||
* secp256k1_nonce_function_bip340 is used, then ndata must be a | ||
* pointer to 32-byte auxiliary randomness as per BIP-340. | ||
* magic: set to SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC at initialization | ||
* and has no other function than making sure the object is | ||
* initialized. | ||
* noncefp: pointer to a nonce generation function. If NULL, | ||
* secp256k1_nonce_function_bip340 is used | ||
* ndata: pointer to arbitrary data used by the nonce generation function | ||
* (can be NULL). If it is non-NULL and | ||
* secp256k1_nonce_function_bip340 is used, then ndata must be a | ||
* pointer to 32-byte auxiliary randomness as per BIP-340. | ||
* s2c_opening: pointer to an secp256k1_schnorrsig_s2c_opening structure which can be | ||
* NULL but is required to be not NULL if this signature creates | ||
* a sign-to-contract commitment (i.e. the `s2c_data32` argument | ||
* is not NULL). | ||
* s2c_data32: pointer to a 32-byte data to create an optional | ||
* sign-to-contract commitment to if not NULL (can be NULL). | ||
*/ | ||
typedef struct { | ||
unsigned char magic[4]; | ||
secp256k1_nonce_function_hardened noncefp; | ||
void *ndata; | ||
} secp256k1_schnorrsig_extraparams; | ||
secp256k1_schnorrsig_s2c_opening* s2c_opening; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding fields to this struct is, I think, an incompatible API break. If a user of the library uses an old version of the header, and links against a new version of the library, I believe the library will access uninitialized fields. Is that right? In theory, that's ok, because we're still in 0.x releases, so any breaks are allowed by SemVer, but this seems unnecessarily burdensome for users. Ideally, extra fields to the struct only affect calls that are impossible to make in old code (e.g. only by new API functions, or when function arguments are set to constants that don't exist in old versions). Idly wondering: can we (ab)use the magic for this? Change the magic in the header, and make the library check whether the old or the new magic is used, and trigger behavior based on that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is right, good catch. There could be multiple solutions:
It seems to me that option 2 is worse, as if the opening magic is not set properly (e.g. by a user using the old headers with the new library), then the function would error. This is better than proceeding silently, but it still is incompatible and forces the user act. The alternative to erroring is to ignore these fields to maintain compatibility, but that would be strange for new library users that want to use the functionality and don't see a proper error message when they fail to initialize the opening struct. Option 3 seems backwards as the point of Option 2 probably makes sense to do for the same reason that the extraparams struct is initialized by the user - to prevent corrupting memory in case the user passes the wrong pointer, and to possibly use it as a version field in the future. TL;DR: I think defining a new magic in the extraparams struct is the best option, and we should also have the caller initialize the As a sidenote: maybe it would be a good to introduce a version check or init function for this library in general, so that if breaking changes are necessary in the future, it could be caught early at runtime. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The extraparams struct was built with the intention that
While this does work in practice, I'm not sure how compliant this is with the C standard. It would access the old extraparams struct through a pointer to a new extraparams struct. Padding shouldn't be an issue here because it only accesses the first member of the struct. It would be better if there was a solution where correctness is more obvious. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If you read the standard strictly, then it's just UB to have conflicting declarations for the same function: https://stackoverflow.com/a/69620952 In practice, and probably even with a more pragmatic reading of the standard, it's unclear what should go wrong. Following your example code, the callee will read the magic value by accessing bytes through an lvalue of type unsigned char. That's always allowed (even if the new struct type has stricter alignment requirements than the old struct type). Moreover, the magic is guaranteed to be at the beginning of the struct, and it's explicitly allowed to use pointers to a struct as pointers to the first element (after an appropriate cast). After determining the version, the callee will read the struct members through an lvalue of the correct type (not identical type with same name but "compatible type"), which is also allowed. As the callee starts with reading characters, it's very much like passing an unsigned char* directly to the function and then casting it a pointer to the right struct type. I think we can do this in practice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the helpful inputs everyone! I added a new commit |
||
const unsigned char* s2c_data32; | ||
} secp256k1_schnorrsig_extraparams_v1; | ||
|
||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC { 0xda, 0x6f, 0xb3, 0x8c } | ||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT {\ | ||
SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC,\ | ||
typedef secp256k1_schnorrsig_extraparams_v1 secp256k1_schnorrsig_extraparams; | ||
|
||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC_V0 { 0xda, 0x6f, 0xb3, 0x8c } | ||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC_V1 { 0x05, 0x96, 0x5b, 0x5c } | ||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC_V1 | ||
|
||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT_V0 {\ | ||
SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC_V0,\ | ||
NULL,\ | ||
NULL\ | ||
} | ||
|
||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT_V1 {\ | ||
SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC_V1,\ | ||
NULL,\ | ||
NULL,\ | ||
NULL,\ | ||
NULL\ | ||
} | ||
#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT_V1 | ||
|
||
/** Create a Schnorr signature. | ||
* | ||
* Does _not_ strictly follow BIP-340 because it does not verify the resulting | ||
|
@@ -183,6 +273,139 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_verify( | |
const secp256k1_xonly_pubkey *pubkey | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(5); | ||
|
||
/** Schnorr Anti-Exfil Protocol | ||
* | ||
* The secp256k1_schnorrsig_anti_exfil_*, functions can be used to prevent a signing device from | ||
* exfiltrating the secret signing keys through biased signature nonces. The general | ||
* idea is that a host provides additional randomness to the signing device client | ||
* and the client commits to the randomness in the nonce using sign-to-contract. | ||
* | ||
* The following scheme is described by Stepan Snigirev here: | ||
* https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-February/017655.html | ||
* and by Pieter Wuille (as "Scheme 6") here: | ||
* https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-March/017667.html | ||
* | ||
* In order to ensure the host cannot trick the signing device into revealing its | ||
* keys, or the signing device to bias the nonce despite the host's contributions, | ||
* the host and client must engage in a commit-reveal protocol as follows: | ||
* 1. The host draws randomness `rho` and computes a sha256 commitment to it using | ||
* `secp256k1_schnorrsig_anti_exfil_host_commit`. It sends this to the signing device. | ||
* 2. The signing device computes a public nonce `R` using the host's commitment | ||
* as auxiliary randomness, using `secp256k1_schnorrsig_anti_exfil_signer_commit`. | ||
* The signing device sends the resulting `R` to the host as a s2c_opening. | ||
* | ||
* If, at any point from this step onward, the hardware device fails, it is | ||
* okay to restart the protocol using **exactly the same `rho`** and checking | ||
* that the hardware device proposes **exactly the same** `R`. Otherwise, the | ||
* hardware device may be selectively aborting and thereby biasing the set of | ||
* nonces that are used in actual signatures. | ||
* | ||
* It takes many (>100) such aborts before there is a plausible attack, given | ||
* current knowledge in 2024. However such aborts accumulate even across a total | ||
* replacement of all relevant devices (but not across replacement of the actual | ||
* signing keys with new independently random ones). | ||
* | ||
* In case the hardware device cannot be made to sign with the given `rho`, `R` | ||
* pair, wallet authors should alert the user and present a very scary message | ||
* implying that if this happens more than even a few times, say 20 or more times | ||
* EVER, they should change hardware vendors and perhaps sweep their coins. | ||
* | ||
* 3. The host replies with `rho` generated in step 1. | ||
* 4. The device signs with `secp256k1_schnorrsig_sign_custom`, using `rho` as `s2c_data32` of the | ||
* extraparams, and sends the signature to the host. | ||
* 5. The host verifies that the signature's public nonce matches the opening from | ||
* step 2 and its original randomness `rho`, using `secp256k1_schnorrsig_anti_exfil_host_verify`. | ||
* | ||
* Rationale: | ||
* - The reason for having a host commitment is to allow the signing device to | ||
* deterministically derive a unique nonce even if the host restarts the protocol | ||
* using the same message and keys. Otherwise the signer might reuse the original | ||
* nonce in two iterations of the protocol with different `rho`, which leaks the | ||
* the secret key. | ||
* - The signer does not need to check that the host commitment matches the host's | ||
* claimed `rho`. Instead it re-derives the commitment (and its original `R`) from | ||
* the provided `rho`. If this differs from the original commitment, the result | ||
* will be an invalid `s2c_opening`, but since `R` was unique there is no risk to | ||
* the signer's secret keys. Because of this, the signing device does not need to | ||
* maintain any state about the progress of the protocol. | ||
*/ | ||
|
||
/** Verify a sign-to-contract commitment. | ||
* | ||
* Returns: 1: the signature contains a commitment to data32 | ||
* 0: incorrect opening | ||
* Args: ctx: a secp256k1 context object, initialized for verification. | ||
* In: sig64: the signature containing the sign-to-contract commitment (cannot be NULL) | ||
* data32: the 32-byte data that was committed to (cannot be NULL) | ||
* opening: pointer to the opening created during signing (cannot be NULL) | ||
*/ | ||
SECP256K1_API int secp256k1_schnorrsig_verify_s2c_commit( | ||
const secp256k1_context* ctx, | ||
const unsigned char *sig64, | ||
const unsigned char *data32, | ||
const secp256k1_schnorrsig_s2c_opening *opening | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); | ||
|
||
|
||
/** Create the initial host commitment to `rho`. Part of the Anti-Exfil Protocol. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here as well, I think it'd be good to add a specification of the Anti-Exfil protocol, and/or references to such a specification. |
||
* | ||
* Returns 1 on success, 0 on failure. | ||
* Args: ctx: pointer to a context object (cannot be NULL) | ||
* Out: rand_commitment32: pointer to 32-byte array to store the returned commitment (cannot be NULL) | ||
* In: rand32: the 32-byte randomness to commit to (cannot be NULL). It must come from | ||
* a cryptographically secure RNG. As per the protocol, this value must not | ||
* be revealed to the client until after the host has received the client | ||
* commitment. | ||
*/ | ||
SECP256K1_API int secp256k1_schnorrsig_anti_exfil_host_commit( | ||
const secp256k1_context* ctx, | ||
unsigned char* rand_commitment32, | ||
const unsigned char* rand32 | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); | ||
|
||
/** Compute signer's original nonce. Part of the Anti-Exfil Protocol. | ||
* | ||
* Returns 1 on success, 0 on failure. | ||
* Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) | ||
* Out: signer_commitment: where the signer's public nonce will be placed. (cannot be NULL) | ||
* In: msg: the message to be signed (cannot be NULL) | ||
* msglen: length of the message | ||
* keypair: pointer to an initialized keypair (cannot be NULL). | ||
* rand_commitment32: the 32-byte randomness commitment from the host (cannot be NULL) | ||
*/ | ||
SECP256K1_API int secp256k1_schnorrsig_anti_exfil_signer_commit( | ||
const secp256k1_context* ctx, | ||
secp256k1_schnorrsig_anti_exfil_signer_commitment* signer_commitment, | ||
const unsigned char *msg, | ||
size_t msglen, | ||
const secp256k1_keypair *keypair, | ||
const unsigned char* rand_commitment32 | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(6); | ||
|
||
/** Verify a signature was correctly constructed using the Anti-Exfil Protocol. | ||
* | ||
* Returns: 1: the signature is valid and contains a commitment to host_data32 | ||
* 0: failure | ||
* Args: ctx: a secp256k1 context object, initialized for verification. | ||
* In: sig64: pointer to the 64-byte signature to verify. | ||
* msg: the message being verified. Can only be NULL if msglen is 0. | ||
* msglen: length of the message | ||
* pubkey: pointer to an x-only public key to verify with (cannot be NULL) | ||
* host_data32: the 32-byte data provided by the host (cannot be NULL) | ||
* signer_commitment: signer commitment produced by `secp256k1_schnorrsig_anti_exfil_signer_commit()`. | ||
* opening: the s2c opening provided by the signer (cannot be NULL) | ||
*/ | ||
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_anti_exfil_host_verify( | ||
const secp256k1_context* ctx, | ||
const unsigned char *sig64, | ||
const unsigned char *msg, | ||
size_t msglen, | ||
const secp256k1_xonly_pubkey *pubkey, | ||
const unsigned char *host_data32, | ||
const secp256k1_schnorrsig_anti_exfil_signer_commitment *signer_commitment, | ||
const secp256k1_schnorrsig_s2c_opening *opening | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(6) SECP256K1_ARG_NONNULL(7) SECP256K1_ARG_NONNULL(8); | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/********************************************************************** | ||
* Copyright (c) 2020 The libsecp256k1-zkp Developers * | ||
* Distributed under the MIT software license, see the accompanying * | ||
* file COPYING or http://www.opensource.org/licenses/mit-license.php.* | ||
**********************************************************************/ | ||
|
||
#ifndef SECP256K1_ECCOMMIT_H | ||
#define SECP256K1_ECCOMMIT_H | ||
|
||
/** Helper function to add a 32-byte value to a scalar */ | ||
static int secp256k1_ec_seckey_tweak_add_helper(secp256k1_scalar *sec, const unsigned char *tweak); | ||
/** Helper function to add a 32-byte value, times G, to an EC point */ | ||
static int secp256k1_ec_pubkey_tweak_add_helper(const secp256k1_ecmult_context* ecmult_ctx, secp256k1_ge *p, const unsigned char *tweak); | ||
|
||
/** Serializes elem as a 33 byte array. This is non-constant time with respect to | ||
* whether pubp is the point at infinity. Thus, you may need to declassify | ||
* pubp->infinity before calling this function. */ | ||
static int secp256k1_ec_commit_pubkey_serialize_const(secp256k1_ge *pubp, unsigned char *buf33); | ||
/** Compute an ec commitment tweak as hash(pubkey, data). */ | ||
static int secp256k1_ec_commit_tweak(unsigned char *tweak32, secp256k1_ge* pubp, secp256k1_sha256* sha, const unsigned char *data, size_t data_size); | ||
/** Compute an ec commitment as pubkey + hash(pubkey, data)*G. */ | ||
static int secp256k1_ec_commit(const secp256k1_ecmult_context* ecmult_ctx, secp256k1_ge* commitp, const secp256k1_ge* pubp, secp256k1_sha256* sha, const unsigned char *data, size_t data_size); | ||
/** Compute a secret key commitment as seckey + hash(pubkey, data). */ | ||
static int secp256k1_ec_commit_seckey(const secp256k1_ecmult_gen_context* ecmult_gen_ctx, secp256k1_scalar* seckey, secp256k1_ge* pubp, secp256k1_sha256* sha, const unsigned char *data, size_t data_size); | ||
/** Verify an ec commitment as pubkey + hash(pubkey, data)*G ?= commitment. */ | ||
static int secp256k1_ec_commit_verify(const secp256k1_ecmult_context* ecmult_ctx, const secp256k1_ge* commitp, const secp256k1_ge* pubp, secp256k1_sha256* sha, const unsigned char *data, size_t data_size); | ||
|
||
#endif /* SECP256K1_ECCOMMIT_H */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took this for a commit from @jonasnick, but I am actually unsure what the purpose is of the magic. Absent programming errors inside the library, could something go wrong if there was no magic field? Are there guidelines of when to use a magic (most structs don't contain any)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the magics are a relative recent addition, and just there for assurance, so that incorrect use can be detected. In correct usage, they serve no function.