Skip to content
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

variable sized precomputed table for signing #337

Merged
merged 1 commit into from Sep 5, 2019

Conversation

@douglasbakkum
Copy link

douglasbakkum commented Oct 18, 2015

This pull request gives an option to reduce the precomputed table size for the signing context (ctx) by setting #define ECMULT_GEN_PREC_BITS [N_BITS].

Motivation: Per #251 and #254, the static table can be reduced to 64kB. However, this is still too big for some of my embedded applications. Setting #define ECMULT_GEN_PREC_BITS 2 produces a 32kB table at a tradeoff of about 75% of the signing speed. Not defining this value will default to the existing implementation of 4 bits. Statistics:

ECMULT_GEN_PREC_BITS = 1
Precomputed table size: 32kB
./bench_sign 
ecdsa_sign: min 195us / avg 200us / max 212us

ECMULT_GEN_PREC_BITS = 2
Precomputed table size: 32kB
./bench_sign 
ecdsa_sign: min 119us / avg 126us / max 134us

ECMULT_GEN_PREC_BITS = 4 (default)
Precomputed table size: 64kB
./bench_sign
ecdsa_sign: min 83.5us / avg 89.6us / max 95.3us

ECMULT_GEN_PREC_BITS = 8
Precomputed table size: 512kB
./bench_sign 
ecdsa_sign: min 96.4us / avg 99.4us / max 104us

Only values of 2 and 4 make sense. 8 bits causes a larger table size with no increase in speed. 1 bit runs, actually, but does not reduce table size and is slower than 2 bits.

@douglasbakkum

This comment has been minimized.

Copy link
Author

douglasbakkum commented Oct 18, 2015

See also comments about alternatively using wNAF:
#189 (comment)

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented Oct 18, 2015

Reasonable enough.

Can you help me understand your memory trade-off requirements? There is a way to halve the table size without largely changing the performance though it requires getting rid of the nums blinding. I'd been under the impression that with the table completely static most of the reasonable target embedded devices (e.g. arm m3s with 192k ram) would end up with the table in their relatively copious flash and there was no real need to reduce below 64k on them.

@douglasbakkum

This comment has been minimized.

Copy link
Author

douglasbakkum commented Oct 18, 2015

Sure. The current device has 256kB of flash and 64kB of RAM on a Cortex M4. The secp256k1 library (including the 64kB table) takes up about 140kB in total.

@jonasschnelli

This comment has been minimized.

Copy link
Contributor

jonasschnelli commented Oct 18, 2015

Concept ACK.
I think hardware wallets and other use-cases on embedded devices don't really seek for super-performance. But they like to use a reliable and well-testes library that fits on a chip with tiny ram and tiny flash. Removing precomputed tables sounds after a logical tradeoff in that territory.

@ddustin

This comment has been minimized.

Copy link

ddustin commented Oct 23, 2018

Is there a reason this was never merged in?

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented May 23, 2019

@real-or-random This complements the WINDOW_G work, would you mind reviewing it?

@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented May 23, 2019

I'll have a look. :)

@@ -49,39 +49,39 @@ static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx

/* compute prec. */
{
secp256k1_gej precj[1024]; /* Jacobian versions of prec. */
secp256k1_gej precj[ECMULT_GEN_PREC_N * ECMULT_GEN_PREC_G]; /* Jacobian versions of prec. */
secp256k1_gej gbase;
secp256k1_gej numsbase;
gbase = gj; /* 16^j * G */

This comment has been minimized.

Copy link
@real-or-random

real-or-random May 24, 2019

Contributor

there are some comments that should be changed here and up to line 67

This comment has been minimized.

Copy link
@douglasbakkum

douglasbakkum May 24, 2019

Author

I updated the comments, assuming you meant to change, for example, 16 to PREC_G. Let me know if there is something in addition.

@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented May 24, 2019

@douglasbakkum I know this is very old. Are you still interested in this PR? If yes, it needs rebase. If not, we could adopt it.

@douglasbakkum

This comment has been minimized.

Copy link
Author

douglasbakkum commented May 24, 2019

Awesome. Yes definitely interested. We have been using this in production BitBox's since the PR was submitted.

I will rebase.

@douglasbakkum douglasbakkum force-pushed the douglasbakkum:master branch from abcaa73 to 1a02d6c May 24, 2019
@douglasbakkum douglasbakkum reopened this May 24, 2019
@douglasbakkum

This comment has been minimized.

Copy link
Author

douglasbakkum commented May 24, 2019

@real-or-random The PR is now rebased. Let me know if you would like further changes.

@douglasbakkum douglasbakkum force-pushed the douglasbakkum:master branch from 4ca3222 to 333fab9 May 24, 2019
@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented May 24, 2019

Thanks, looks good to me!

What we need to do is make this and #596 consistent (for naming of constants, ./configure, etc.) I think the easiest way forward is to get #596 merged and then adapt this PR here. (I know this is also the easiest way for me as the author of #596 but I'm of course open to other suggestions too. :P)

@gmaxwell What do you think? I'm not convinced that "window size" would be an appropriate term here, so maybe PREC_BITS makes more sense in general?

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented May 25, 2019

Sounds fine to me (both on the sequence and the naming)

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented May 29, 2019

#596 is now merged and I don't believe there is anything at risk of imminent merger which will cause difficult conflicts with this PR.

@douglasbakkum douglasbakkum force-pushed the douglasbakkum:master branch from 333fab9 to bd0d0fb May 29, 2019
@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented May 29, 2019

@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented May 30, 2019

Great, so if ECMULT_GEN_PREC_BITS is fine, then this constant should be ./configure-able as in #596. See this commit for reference: 2842dc5

@douglasbakkum Do you want to give it a try?

@douglasbakkum

This comment has been minimized.

Copy link
Author

douglasbakkum commented May 31, 2019

@real-or-random Yea, agree. If it is quick for you, would you mind doing so?

@vhnatyk

This comment has been minimized.

Copy link

vhnatyk commented Jun 19, 2019

Hi! any progress with this?

@douglasbakkum

This comment has been minimized.

Copy link
Author

douglasbakkum commented Jun 21, 2019

Sorry for the delays. My colleague @benma made a commit to make the value configurable.

@douglasbakkum douglasbakkum force-pushed the douglasbakkum:master branch from 9d270c1 to df3224d Jun 21, 2019
@benma

This comment has been minimized.

Copy link
Contributor

benma commented Jun 29, 2019

@real-or-random did you have a chance to test the configuration?

@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented Jun 29, 2019

No, sorry, not yet. It's on my long "GitHub saved notifications" list, I'll have a look next week. :)

@@ -45,23 +46,23 @@ int main(int argc, char **argv) {
fprintf(fp, "#define _SECP256K1_ECMULT_STATIC_CONTEXT_\n");
fprintf(fp, "#include \"src/group.h\"\n");
fprintf(fp, "#define SC SECP256K1_GE_STORAGE_CONST\n");
fprintf(fp, "static const secp256k1_ge_storage secp256k1_ecmult_static_context[64][16] = {\n");
fprintf(fp, "static const secp256k1_ge_storage secp256k1_ecmult_static_context[ECMULT_GEN_PREC_N][ECMULT_GEN_PREC_G] = {\n");

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 3, 2019

Contributor

This is somewhat weird if you change the config in between but for some reason gen_context is not run again.

So I think the generated ecmult_static_context.h should have the gen_context.c compile-time values of ECMULT_GEN_PREC_N and ECMULT_GEN_PREC_G hardcoded and contain a precompiler assertion (#error) that verifies they're identical to the ecmult_static_context.h-compile-time values. This should make sure that we never compile with a ecmult_static_context.h generated with a different configuration. (Alternatively, we could make the file name of ecmult_static_context.h make dependent on the configured values, e.g., ecmult_static_context_64_16.h or something.)

But maybe I make a mistake here, it's easy to get these things wrong. So correct me if you think I'm wrong.

This comment has been minimized.

Copy link
@benma

benma Jul 4, 2019

Contributor

That's a great catch. Normally gen_context is run with make if the configuration changed, but it might not be absolutely failsafe (e.g. by manually overwriting the file with a different version etc.).

Change incoming.

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

Thanks, looks good!
Can you add a sentence like "Try deleting ecmult_static_context.h before the build." or similar?

@vhnatyk

This comment has been minimized.

Copy link

vhnatyk commented Jul 12, 2019

Seems we are so close 😄, sorry for spamming - what else needs to be done?

@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented Jul 12, 2019

Ah I haven't seen the changes. I'll have a look next week!

Copy link
Contributor

real-or-random left a comment

With ./configure --disable-openssl-tests --with-ecmult-gen-precision=8 (but not 2 and 4) and valgrind ./exhaustive_tests I get millions of warnings like

==6831== Invalid read of size 16
==6831==    at 0x1111AB: secp256k1_fe_add (field_5x52_impl.h:405)
==6831==    by 0x1111AB: secp256k1_gej_double_var.part.0.constprop.0 (group_impl.h:358)
==6831==    by 0x1154ED: secp256k1_gej_double_var (group_impl.h:326)
==6831==    by 0x1154ED: secp256k1_ecmult_gen_context_build (ecmult_gen_impl.h:83)
==6831==    by 0x11A55E: secp256k1_context_preallocated_create (secp256k1.c:130)
==6831==    by 0x11A55E: secp256k1_context_create (secp256k1.c:142)
==6831==    by 0x109130: main (tests_exhaustive.c:460)
==6831==  Address 0x1ffedfe740 is on thread 1's stack
==6831==  in frame #1, created by secp256k1_ecmult_gen_context_build (group_impl.h:29)
==6831==
==6831== Invalid read of size 16
==6831==    at 0x1111B0: secp256k1_fe_add (field_5x52_impl.h:405)
==6831==    by 0x1111B0: secp256k1_gej_double_var.part.0.constprop.0 (group_impl.h:358)
==6831==    by 0x1154ED: secp256k1_gej_double_var (group_impl.h:326)
==6831==    by 0x1154ED: secp256k1_ecmult_gen_context_build (ecmult_gen_impl.h:83)
==6831==    by 0x11A55E: secp256k1_context_preallocated_create (secp256k1.c:130)
==6831==    by 0x11A55E: secp256k1_context_create (secp256k1.c:142)
==6831==    by 0x109130: main (tests_exhaustive.c:460)
==6831==  Address 0x1ffedfe730 is on thread 1's stack
==6831==  in frame #1, created by secp256k1_ecmult_gen_context_build (group_impl.h:29)
==6831==
==6831== Invalid write of size 4
==6831==    at 0x1111B5: secp256k1_fe_add (field_5x52_impl.h:412)
==6831==    by 0x1111B5: secp256k1_gej_double_var.part.0.constprop.0 (group_impl.h:358)
==6831==    by 0x1154ED: secp256k1_gej_double_var (group_impl.h:326)
==6831==    by 0x1154ED: secp256k1_ecmult_gen_context_build (ecmult_gen_impl.h:83)
==6831==    by 0x11A55E: secp256k1_context_preallocated_create (secp256k1.c:130)
==6831==    by 0x11A55E: secp256k1_context_create (secp256k1.c:142)
==6831==    by 0x109130: main (tests_exhaustive.c:460)
==6831==  Address 0x1ffedfe75c is on thread 1's stack
==6831==  in frame #1, created by secp256k1_ecmult_gen_context_build (group_impl.h:29)
==6831==
==6831== Invalid write of size 8
==6831==    at 0x1111C8: secp256k1_fe_add (field_5x52_impl.h:405)
==6831==    by 0x1111C8: secp256k1_gej_double_var.part.0.constprop.0 (group_impl.h:358)
==6831==    by 0x1154ED: secp256k1_gej_double_var (group_impl.h:326)
==6831==    by 0x1154ED: secp256k1_ecmult_gen_context_build (ecmult_gen_impl.h:83)
==6831==    by 0x11A55E: secp256k1_context_preallocated_create (secp256k1.c:130)
==6831==    by 0x11A55E: secp256k1_context_create (secp256k1.c:142)
==6831==    by 0x109130: main (tests_exhaustive.c:460)
==6831==  Address 0x1ffedfe740 is on thread 1's stack
==6831==  in frame #1, created by secp256k1_ecmult_gen_context_build (group_impl.h:29)

I haven't looked at this further.

AC_ARG_WITH([ecmult-gen-precision], [AS_HELP_STRING([--with-ecmult-gen-precision=2|4|8|auto],
[Precision bits to tune the precomputed table size for signing.]
[The size of the table is 32kB for 2 bits, 64kB for 4 bits, 512kB for 8 bits of precision.]
[A smaller table size usually results in slower signing.]

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

nit: maybe phrase it the around way around: "A larger table size usually results in possible faster signing."

(to be consistent with "Larger values result in possibly better performance" for the other config entry)

This comment has been minimized.

Copy link
@benma

benma Jul 15, 2019

Contributor

With ./configure --disable-openssl-tests --with-ecmult-gen-precision=8 (but not 2 and 4) and valgrind ./exhaustive_tests I get millions of warnings like

There is no issue if you call it like this:

valgrind --max-stackframe=2097912 ./exhaustive_tests

Reference:

-max-stackframe= [default: 2000000]
The maximum size of a stack frame. If the stack pointer moves by more than this amount then Valgrind will assume that the program is switching to a different stack.
You may need to use this option if your program has large stack-allocated arrays.

Seems like the big option just needs more stack space. It's a valgrind heuristic only and the tests run fine otherwise.

Will add a note to the README.

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

Oh indeed, thanks!

@@ -45,23 +46,23 @@ int main(int argc, char **argv) {
fprintf(fp, "#define _SECP256K1_ECMULT_STATIC_CONTEXT_\n");
fprintf(fp, "#include \"src/group.h\"\n");
fprintf(fp, "#define SC SECP256K1_GE_STORAGE_CONST\n");
fprintf(fp, "static const secp256k1_ge_storage secp256k1_ecmult_static_context[64][16] = {\n");
fprintf(fp, "static const secp256k1_ge_storage secp256k1_ecmult_static_context[ECMULT_GEN_PREC_N][ECMULT_GEN_PREC_G] = {\n");

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

Thanks, looks good!
Can you add a sentence like "Try deleting ecmult_static_context.h before the build." or similar?

Makefile.am Outdated
@@ -172,7 +172,7 @@ src/ecmult_static_context.h: $(gen_context_BIN)
CLEANFILES = $(gen_context_BIN) src/ecmult_static_context.h $(JAVAROOT)/$(JAVAORG)/*.class .stamp-java
endif

EXTRA_DIST = autogen.sh src/gen_context.c src/basic-config.h $(JAVA_FILES)
EXTRA_DIST = autogen.sh src/gen_context.c src/libsecp256k1-config.h src/basic-config.h $(JAVA_FILES)

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

I don't think we want to distribute src/libsecp256k1-config.h. It will be generated by configure.

This comment has been minimized.

Copy link
@benma

benma Jul 15, 2019

Contributor

Can you add a sentence like "Try deleting ecmult_static_context.h before the build." or similar?

Done.

I don't think we want to distribute src/libsecp256k1-config.h. It will be generated by configure.

That was a misguided attempt at having gen_context rebuild the static context upon configuration change. It seems to work now anyway, so I removed it again.

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

Oh hm, apparently make distcheck fails here:
https://travis-ci.org/bitcoin-core/secp256k1/jobs/558864214#L847

On a second thought, I'm not sure that including src/libsecp256k1-config.h is a good idea. There are downstream users of this library (e.g., https://github.com/rust-bitcoin/rust-secp256k1) that don't use the autotools. But I'm not sure what the best thing is. Maybe including it only if ECMULT_GEN_PREC_BITS is not defined. Sounds pragmatic to me.

This comment has been minimized.

Copy link
@benma

benma Jul 15, 2019

Contributor

fixed

Copy link
Contributor

real-or-random left a comment

Sorry for bringing up more and more stuff. We should be really close now. :)

README.md Outdated
With valgrind, you might need to increase the max stack size:
$ valgrind --max-stackframe=2097912 ./exhaustive_tests

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

I need slightly larger values.... Maybe recommend 2500000?

This comment has been minimized.

Copy link
@benma

benma Jul 15, 2019

Contributor

Done

@@ -4,9 +4,10 @@
* file COPYING or http://www.opensource.org/licenses/mit-license.php.*
**********************************************************************/

#include "libsecp256k1-config.h" // for ECMULT_GEN_PREC_BITS

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

It's not crucial but we should make sure that this warning disappears:
/configure --disable-openssl-tests --with-ecmult-gen-precision=8 --enable-experimental --enable-module-recovery --enable-module-ecdh --with-ecmult-window=22 CC=clang

In file included from src/gen_context.c:9:
src/basic-config.h:33: warning: "ECMULT_WINDOW_SIZE" redefined
   33 | #define ECMULT_WINDOW_SIZE 15
      |
In file included from src/gen_context.c:7:
src/libsecp256k1-config.h:18: note: this is the location of the previous definition
   18 | #define ECMULT_WINDOW_SIZE 22

I guess other compilers warn, too.

In the end it does not matter what ECMULT_WINDOW_SIZE is for use in gen_context.c. I think sticking to 15 is reasonable (but the fact that we ignore the config setting here should be more prominent in the source).

This comment has been minimized.

Copy link
@real-or-random

real-or-random Jul 15, 2019

Contributor

In the end it does not matter what ECMULT_WINDOW_SIZE is for use in gen_context.c. I think sticking to 15 is reasonable (but the fact that we ignore the config setting here should be more prominent in the source).

Actually it does not matter too much because gen_context does not use ECMULT_WINDOW_SIZE at all.

This comment has been minimized.

Copy link
@benma

benma Jul 15, 2019

Contributor

Fixed via #undef ECMULT_WINDOW_SIZE in basic-config.h, like with the other definitions.

This comment has been minimized.

Copy link
@vhnatyk

vhnatyk Jul 22, 2019

@real-or-random sorry to bug, seems ok now? 😊

This comment has been minimized.

Copy link
@benma

benma Sep 4, 2019

Contributor

Ping @real-or-random - seems we are really close now :P

@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented Sep 4, 2019

Yes, we're close. :)

ACK 1d26b27 (mod squashing) I read the changes, and tested them both with and without static precomputation, with different precision values and also with valgrind.

Can you squash some of the commits?

@real-or-random real-or-random requested a review from sipa Sep 4, 2019
@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented Sep 4, 2019

@sipa Can you have look at this? I think you know the algorithm best.

@sipa

This comment has been minimized.

Copy link
Contributor

sipa commented Sep 4, 2019

Code review ACK 1d26b27. Looks great; all comments I wanted to make were addressed in later commits.

Please squash the fixup commits.

@sipa
sipa approved these changes Sep 4, 2019
@elichai

This comment has been minimized.

Copy link
Contributor

elichai commented Sep 4, 2019

Maybe better docs to differentiate the different between ecmult_static_precomputation,ecmult-window,ecmult-gen-precision?

@benma

This comment has been minimized.

Copy link
Contributor

benma commented Sep 4, 2019

@elichai okay to do this in a different PR? Not strictly related.

@real-or-random

This comment has been minimized.

Copy link
Contributor

real-or-random commented Sep 5, 2019

I think this can happen in a different PR.

@elichai

This comment has been minimized.

Copy link
Contributor

elichai commented Sep 5, 2019

Sure, just getting to a lot of tables that it's getting confusing which table will affect what.

make ECMULT_GEN_PREC_BITS configurable

ecmult_static_context.h: add compile time config assertion (#3) - Prevents accidentally using a file which was generated with a
different configuration.

README: mention valgrind issue

With --with-ecmult-gen-precision=8, valgrind needs a max stack size
adjustment to not run into a stack switching heuristic:

http://valgrind.org/docs/manual/manual-core.html

> -max-stackframe= [default: 2000000]
> The maximum size of a stack frame. If the stack pointer moves by more than this amount then Valgrind will assume that the program is switching to a different stack.
You may need to use this option if your program has large stack-allocated arrays.

basic-config: undef ECMULT_WINDOW_SIZE before (re-)defining it
@douglasbakkum douglasbakkum force-pushed the douglasbakkum:master branch from 1d26b27 to dcb2e3b Sep 5, 2019
@douglasbakkum

This comment has been minimized.

Copy link
Author

douglasbakkum commented Sep 5, 2019

Thanks all.
The commits are now squashed and rebased.

Copy link
Contributor

real-or-random left a comment

ACK dcb2e3b verified that all changes to the previous ACKed 1d26b27 were due to the rebase

Copy link
Contributor

jonasnick left a comment

ACK dcb2e3b read the code and tested various configurations with valgrind

real-or-random added a commit that referenced this pull request Sep 5, 2019
dcb2e3b variable signing precompute table (djb)

Pull request description:

  This pull request gives an option to reduce the precomputed table size for the signing context (`ctx`) by setting `#define ECMULT_GEN_PREC_BITS [N_BITS]`.

  Motivation: Per #251 and #254, the static table can be reduced to 64kB. However, this is still too big for some of my embedded applications. Setting `#define ECMULT_GEN_PREC_BITS 2` produces a 32kB table at a tradeoff of about 75% of the signing speed. Not defining this value will default to the existing implementation of 4 bits. Statistics:

  ```
  ECMULT_GEN_PREC_BITS = 1
  Precomputed table size: 32kB
  ./bench_sign
  ecdsa_sign: min 195us / avg 200us / max 212us

  ECMULT_GEN_PREC_BITS = 2
  Precomputed table size: 32kB
  ./bench_sign
  ecdsa_sign: min 119us / avg 126us / max 134us

  ECMULT_GEN_PREC_BITS = 4 (default)
  Precomputed table size: 64kB
  ./bench_sign
  ecdsa_sign: min 83.5us / avg 89.6us / max 95.3us

  ECMULT_GEN_PREC_BITS = 8
  Precomputed table size: 512kB
  ./bench_sign
  ecdsa_sign: min 96.4us / avg 99.4us / max 104us
  ```

  Only values of 2 and 4 make sense. 8 bits causes a larger table size with no increase in speed. 1 bit runs, actually, but does not reduce table size and is slower than 2 bits.

ACKs for top commit:
  real-or-random:
    ACK dcb2e3b verified that all changes to the previous ACKed 1d26b27 were due to the rebase
  jonasnick:
    ACK dcb2e3b read the code and tested various configurations with valgrind

Tree-SHA512: ed6f68ca23ffdc4b59d51525336b34b25521233537edbc74d32dfb3eafd8196419be17f01cbf10bd8d87ce745ce143085abc6034727f742163f7e5f13f26f56e
@real-or-random real-or-random merged commit dcb2e3b into bitcoin-core:master Sep 5, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vhnatyk

This comment has been minimized.

Copy link

vhnatyk commented Sep 5, 2019

So happy this got merged finally!) Hopefully I can start getting rid of black magik in my code now))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.