secp256k1_scalar_{zero,one}
(apparently introduced in 34a67c773b0871e5797c7ab506d004e80911f120, PR #710) can be directly used instead for the values 0 or 1. There is very likely not even a difference in run-time, but it leads to simpler and less code which might be nice.
refactor: take use of secp256k1_scalar_{zero,one}
constants
#1330
pull
theStack
wants to merge
2
commits into
bitcoin-core:master
from
theStack:use_zero_one_scalar_constants
changing
4
files
+53 −62
-
theStack commented at 11:19 pm on May 29, 2023: contributorRather than allocating a (non-constant) scalar variable on the stack with the sole purpose of setting it to a constant value, the global constants
-
in src/tests.c:4639 in 94b85556a6 outdated
4635@@ -4643,7 +4636,7 @@ static int ecmult_multi_false_callback(secp256k1_scalar *sc, secp256k1_ge *pt, s 4636 4637 static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi_func ecmult_multi) { 4638 int ncount; 4639- secp256k1_scalar szero; 4640+ const secp256k1_scalar szero = secp256k1_scalar_zero;
real-or-random commented at 7:36 am on May 30, 2023:Why not get rid ofszero
entirely?
theStack commented at 10:28 am on May 30, 2023:Kept it due to some irrational “diff will get too large, lines will get too long” thoughts and a portion of laziness 😅 Done now. -
in src/tests.c:1 in 94b85556a6
real-or-random commented at 7:38 am on May 30, 2023:Can you add tests that check that the constants match what you get withsecp256k1_scalar_set_int
?
theStack commented at 10:30 am on May 30, 2023:Thanks, done. Also putsecp256k1_scalar_is_{zero,one}
checks in. -
real-or-random commented at 7:38 am on May 30, 2023: contributorConcept ACK
-
refactor: take use of `secp256k1_scalar_{zero,one}` constants 654246c635
-
tests: add checks for scalar constants `secp256k1_scalar_{zero,one}` ade5b36701
-
theStack force-pushed on May 30, 2023
-
real-or-random approved
-
real-or-random commented at 11:53 am on May 30, 2023: contributorutACK ade5b367018a624ff7ca1ecbb4a64889d47b0142
-
real-or-random added the label refactor on May 30, 2023
-
sipa commented at 4:49 pm on May 31, 2023: contributorutACK ade5b367018a624ff7ca1ecbb4a64889d47b0142
-
real-or-random merged this on May 31, 2023
-
real-or-random closed this on May 31, 2023
-
theStack deleted the branch on May 31, 2023
-
vmta referenced this in commit e1120c94a1 on Jun 4, 2023
-
sipa referenced this in commit 901336eee7 on Jun 21, 2023
-
vmta referenced this in commit 8f03457eed on Jul 1, 2023
-
hebasto referenced this in commit 270d2b37b8 on Jul 21, 2023
-
jonasnick cross-referenced this on Jul 26, 2023 from issue Upstream PRs 1314, 1317, 1318, 1316, 1327, 1310, 1328, 1333, 1330, 1334, 1337, 1341, 1339, 1350, 1349, 1338, 1129, 1347, 1336, 1295, 1354, 1355, 1356 by jonasnick
-
theStack cross-referenced this on Aug 1, 2023 from issue refactor: take use of `secp256k1_scalar_{zero,one}` constants (part 2) by theStack
Labels
refactor/smell