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

Replace CSHA256::FinalizeNoPadding with CSHA256::Compress #819

Open
daira opened this issue Apr 1, 2016 · 2 comments
Open

Replace CSHA256::FinalizeNoPadding with CSHA256::Compress #819

daira opened this issue Apr 1, 2016 · 2 comments
Assignees
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. Zcash codebase

Comments

@daira
Copy link
Contributor

daira commented Apr 1, 2016

In #814 we fixed a potential usage error of the SHA-256 hash API when used to obtain the compression function, by requiring 64 bytes to have been written (if a particular flag is passed in). However the resulting construct/Write/FinalizeNoPadding API is unnecessarily complex and verbose.

Replace it with a simple function:

static void CSHA256::Compress(unsigned char (&hash)[OUTPUT_SIZE], const unsigned char (&input)[BLOCK_SIZE]);

(Not 100% sure I have the reference syntax correct there; the intent is a reference to an array of known size, which is a const array in the case of input.)

@daira daira added Zcash codebase C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels Apr 1, 2016
@defuse
Copy link
Contributor

defuse commented Apr 2, 2016

I suggested this to @ebfull in our pairing and he said he wanted to be able to use .Write() with the compression function (e.g. to write the ask first and the other stuff next).

I actually think we should do this anyway, because then we aren't relying on .Write() and the complicated check (bytes == 64) to make sure the compression function is actually getting called.

@daira
Copy link
Contributor Author

daira commented Apr 4, 2016

I considered that, but I think it is mixing up the concatenation of byte strings with the application of the hash. I would much prefer that we had a concise (and more functional in style) way to do the former, independent of the latter. We do concatenation in many more cases than we do hashing.

str4d added a commit to str4d/zcash that referenced this issue Oct 21, 2020
c6b6b8f1b Merge zcash#830: Rip out non-endomorphism code + dependencies
c582abade Consistency improvements to the comments
63c6b7161 Reorder comments/function around scalar_split_lambda
2edc514c9 WNAF of lambda_split output has max size 129
4232e5b7d Rip out non-endomorphism code
ebad8414b Check correctness of lambda split without -DVERIFY
fe7fc1fda Make lambda constant accessible
9d2f2b44d Add tests to exercise lambda split near bounds
9aca2f7f0 Add secp256k1_split_lambda_verify
acab934d2 Detailed comments for secp256k1_scalar_split_lambda
76ed922a5 Increase precision of g1 and g2
6173839c9 Switch to our own memcmp function
63150ab4d Merge zcash#827: Rename testrand functions to have test in name
c5257aed0 Merge zcash#821: travis: Explicitly set --with-valgrind
bb1f54280 Merge zcash#818: Add static assertion that uint32_t is unsigned int or wider
a45c1fa63 Rename testrand functions to have test in name
5006895bd Merge zcash#808: Exhaustive test improvements + exhaustive schnorrsig tests
4eecb4d6e travis: VALGRIND->RUN_VALGRIND to avoid confusion with WITH_VALGRIND
66a765c77 travis: Explicitly set --with-valgrind
d7838ba6a Merge zcash#813: Enable configuring Valgrind support
7ceb0b761 Merge zcash#819: Enable -Wundef warning
8b7dcdd95 Add exhaustive test for extrakeys and schnorrsig
08d7d8929 Make pubkey parsing test whether points are in the correct subgroup
87af00b51 Abstract out challenge computation in schnorrsig
63e1b2aa7 Disable output buffering in tests_exhaustive.c
39f67dd07 Support splitting exhaustive tests across cores
e99b26fcd Give exhaustive_tests count and seed cmdline inputs
49e6630bc refactor: move RNG seeding to testrand
b110c106f Change exhaustive test groups so they have a point with X=1
cec7b18a3 Select exhaustive lambda in function of order
78f6cdfaa Make the curve B constant a secp256k1_fe
d7f39ae4b Delete gej_is_valid_var: unused outside tests
8bcd78cd7 Make secp256k1_scalar_b32 detect overflow in scalar_low
c498366e5 Move exhaustive tests for recovery to module
be3179154 Make group order purely compile-time in exhaustive tests
e73ff3092 Enable -Wundef warning
c0041b5cf Add static assertion that uint32_t is unsigned int or wider
4ad408faf Merge zcash#782: Check if variable=yes instead of if var is set in travis.sh
412bf874d configure: Allow specifying --with[out]-valgrind explicitly
34debf7a6 Modify .travis.yml to explictly pass no in env vars instead of setting to nothing
a0e99fc12 Merge zcash#814: tests: Initialize random group elements fully
5738e8622 tests: Initialize random group elements fully
c9939ba55 Merge zcash#812: travis: run bench_schnorrsig
a51f2af62 travis: run bench_schnorrsig
ef37761fe Change travis.sh to check if variables are equal to yes instead of not-empty. Before this, setting `VALGRIND=wat` was considered as true, and to make it evaluate as false you had to unset the variable `VALGRIND=` but not it checks if `VALGRIND=yes` and if it's not `yes` then it's evaluated to false

git-subtree-dir: src/secp256k1
git-subtree-split: c6b6b8f1bb044d7d1aa065ebb674adde98a36a8e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. Zcash codebase
Projects
None yet
Development

No branches or pull requests

3 participants