add static context object which has no capabilities #553
pull apoelstra wants to merge 2 commits into bitcoin-core:master from apoelstra:static-context changing 3 files +20 −0-
apoelstra commented at 9:27 pm on August 15, 2018: contributor
-
apoelstra force-pushed on Aug 15, 2018
-
apoelstra cross-referenced this on Aug 20, 2018 from issue remove `Secp256k1::without_caps` from public API and all no-capability context args by apoelstra
-
gmaxwell commented at 3:59 pm on September 4, 2018: contributorShould we document that you can’t call set_illegal_callback on this and perhaps add some checks on some of the disallowed calls to fail if you try?
-
apoelstra commented at 4:02 pm on September 4, 2018: contributorSure, we can do a
VERIFY_CHECK
on those functions that thectx
is not the static one, much like how we compare to NULL now. -
apoelstra commented at 5:53 pm on September 20, 2018: contributor
Added a commit which
VERIFY_CHECK
s on all functions that modify the context object.I considered using
ARG_CHECK
but I didn’t see the point, since you can’t change the callback function from its default CHECK-like behaviour. Also, since it is actually UB to call these functions on static data (without synchronization primitives anyway), I wanted to prevent it more emphatically.Edit Changed from VERIFY_CHECK to CHECK because Greg pointed out that VERIFY is only defined during unit testing.
-
apoelstra force-pushed on Sep 20, 2018
-
apoelstra cross-referenced this on Sep 22, 2018 from issue Eliminate scratch memory used when generating contexts by apoelstra
-
in include/secp256k1.h:187 in 9461ae6f02 outdated
178@@ -179,6 +179,13 @@ typedef int (*secp256k1_nonce_function)( 179 #define SECP256K1_TAG_PUBKEY_HYBRID_EVEN 0x06 180 #define SECP256K1_TAG_PUBKEY_HYBRID_ODD 0x07 181 182+/** A simple secp256k1 context object with no precomputed tables. These are useful for 183+ * type serialization/parsing functions which require a context object to maintain 184+ * API consistency, but currently do not require expensive precomputations or dynamic 185+ * allocations. 186+ */ 187+SECP256K1_API extern secp256k1_context *secp256k1_context_no_precomp;
sipa commented at 5:01 am on October 4, 2018:Can you mark this objectconst
? If that works you don’t need any of theCHECK
invocations I think as they all require a non-const argument. It would also alleviate any concerns about synchronization etc.
apoelstra commented at 3:17 pm on October 4, 2018:Done -
add static context object which has no capabilities ed7c08417a
-
prevent attempts to modify `secp256k1_context_no_precomp` 40fde611bd
-
apoelstra force-pushed on Oct 4, 2018
-
in src/secp256k1.c:102 in 40fde611bd
98@@ -91,6 +99,7 @@ secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { 99 } 100 101 void secp256k1_context_destroy(secp256k1_context* ctx) { 102+ CHECK(ctx != secp256k1_context_no_precomp);
sipa commented at 7:56 pm on October 16, 2018:Are these necessary? A non-const
ctx pointer is used, which can’t besecp256k1_context_no_precomp
(as that one is const).
apoelstra commented at 8:23 pm on October 16, 2018:Oh, good point. I guess they’re not necessary. I can remove them if you want, though I don’t mind having them there. -
sipa commented at 7:17 pm on October 17, 2018: contributorutACK 40fde611bd145bdafb78a52af6d197489101bafb
-
real-or-random commented at 10:33 am on October 18, 2018: contributorACK 40fde611bd145bdafb78a52af6d197489101bafb
-
apoelstra cross-referenced this on Oct 27, 2018 from issue 0.13 tracking issue by apoelstra
-
sipa merged this on Nov 6, 2018
-
sipa closed this on Nov 6, 2018
-
sipa referenced this in commit 314a61d724 on Nov 6, 2018
-
dongcarl cross-referenced this on Nov 6, 2018 from issue PublicKey de/serialization should support io::{Read,Write} traits. by dongcarl
-
sipa referenced this in commit e34ceb333b on Nov 26, 2018
-
etscrivner cross-referenced this on Jan 5, 2019 from issue Investigate using static context objects by etscrivner
-
gmaxwell cross-referenced this on Jan 23, 2021 from issue UB in tests: Invalid pointer comparison in secp256k1_fe_inv_all_var by practicalswift
-
sipa cross-referenced this on Jan 24, 2021 from issue Remove unused secp256k1_fe_inv_all_var by sipa