This PR compares Bitcoin Core's implementation of Poly1305 with Floodyberry's public domain implementation in order to find implementation discrepancies if any.
Instructions to test this PR: Step1: Fetch the pull request Step2: Switch to the pull request branch and run the following commands:
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ FUZZ=crypto_diff_fuzz_poly1305 src/test/fuzz/fuzz
17 | +static INLINE void fU32TO8_LE_FAST(uint8_t* p, const uint32_t v) { *(uint32_t*)p = v; } 18 | + 19 | +#define U8TO32_LE(p) fU8TO32_LE_FAST(p) 20 | +#define U32TO8_LE(p, v) fU32TO8_LE_FAST(p, v) 21 | + 22 | +void poly1305_auth(unsigned char out[16], const unsigned char* m, size_t inlen, const unsigned char key[32])
If you include third party code, can you please add a comment that mentions
Thanks for the review I have made the necessary changes, and PR is ready for further review.
Thanks!
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | practicalswift |
| Stale ACK | laanwj, stratospher |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
No conflicts as of last run.
Concept ACK on introducing more differential fuzzing
Ideas for further differential fuzzing:
CRIPEMD160, CSHA1, CSHA256, CHash160 and CHash256: all which are used in EvalScript.src/crypto/sha256.cpp (internal implementation) vs src/crypto/sha256_{avx2,shani,sse41,sse4}.cpp. The self-test SelfTest() in src/crypto/sha256.cpp might serve as inspiration.
Code review ACK d2af151e3cb638110087d905a31c205f4d81f7bc
Co-authored-by: Ruhi <44024636+stratospher@users.noreply.github.com>
Co-authored-by: Ruhi <44024636+stratospher@users.noreply.github.com>
Co-authored-by: Ruhi <44024636+stratospher@users.noreply.github.com>
Are you still working on this?
Yes, I'm still working on this. I'm looking for concept ACKs for this approach. Also wondering whether differential fuzzing between c++ and python implementations would be more valuable? (See #23441 (comment) also.)
I'll update soon regarding the ci fuzz crash. EDIT: it's only missing some sanitizer suppressions.
I don't think differential fuzzing with a Python implementation is worth it (much slower, and much harder due to cross-language comparing). There is also nothing particularly authoritative about any given Python implementation; in the end it's comparison with actually-deployed poly1305 code we want.
I don't think differential fuzzing with a Python implementation is worth it (much slower, and much harder due to cross-language comparing). There is also nothing particularly authoritative about any given Python implementation; in the end it's comparison with actually-deployed poly1305 code we want.
I was thinking of the python implementation in the functional testing framework but this makes sense. Closing this PR until we find a better way to test.
FWIW, I didn't mean to suggest to close this PR. I meant that I don't think fuzzing against a Python implementation adds anything compared to what this PR was aiming to do.
thanks for clarifying! I find the reference and current implementation to be similar for a good differential fuzz test now. I'd like to understand more about how differential fuzzing is done in other projects before continuing with this PR.