d543c0d917 Merge bitcoin-core/secp256k1#1734: Introduce (mini) unit test framework f44c1ebd96 Merge bitcoin-core/secp256k1#1719: ci: DRY workflow using anchors a44a339384 Merge bitcoin-core/secp256k1#1750: ci: Use clang-snapshot in "MSan" job 15d014804e ci: Drop default for `inputs.command` in `run-in-docker-action` 1decc49a1f ci: Use YAML anchor and aliases for repeated "CI script" steps dff1bc107d ci, refactor: Generalize use of `matrix.configuration.env_vars` 4b644da199 ci: Use YAML anchor and aliases for repeated "Print logs" steps a889cd93df ci: Bump `actions/checkout` version 574c2f3080 ci: Use YAML anchor and aliases for repeated "Checkout" steps 53585f93b7 ci: Use clang-snapshot in "MSan" job 6894c964f3 Fix Clang 21+ `-Wuninitialized-const-pointer` warning when using MSan 2b7337f63a Merge bitcoin-core/secp256k1#1756: ci: Fix image caching and apply other improvements f163c35897 ci: Set `DEBIAN_FRONTEND=noninteractive` 70ae177ca0 ci: Bump `docker/build-push-action` version b2a95a420f ci: Drop `tags` input for `docker/build-push-action` 122014edb3 ci: Add `scope` parameter to `cache-{to,from}` options 2f4546ce56 test: add --log option to display tests execution 95b9953ea4 test: Add option to display all available tests 953f7b0088 test: support running specific tests/modules targets 0302c1a3d7 test: add --help for command-line options 9ec3bfe22d test: adapt modules to the new test infrastructure 48789dafc2 test: introduce (mini) unit test framework baa265429f Merge bitcoin-core/secp256k1#1727: docs: Clarify that callback can be called more than once 4d90585fea docs: Improve API docs of _context_set_illegal_callback 895f53d1cf docs: Clarify that callback can be called more than once de6af6ae35 Merge bitcoin-core/secp256k1#1748: bench: improve context creation in ECDH benchmark 5817885153 Merge bitcoin-core/secp256k1#1749: build: Fix warnings in x86_64 assembly check ab560078aa build: Fix warnings in x86_64 assembly check 10dab907e7 Merge bitcoin-core/secp256k1#1741: doc: clarify API doc of `secp256k1_ecdsa_recover` return value dfe284ed2d bench: improve context creation in ECDH benchmark 7321bdf27b doc: clarify API doc of `secp256k1_ecdsa_recover` return value b475654302 Merge bitcoin-core/secp256k1#1745: test: introduce group order byte-array constant for deduplication 9cce703863 refactor: move 'gettime_i64()' to tests_common.h 0c91c56041 test: introduce group order byte-array constant for deduplication 88be4e8d86 Merge bitcoin-core/secp256k1#1735: musig: Invalidate secnonce in secp256k1_musig_partial_sign 36e76952cb Merge bitcoin-core/secp256k1#1738: check-abi: remove support for obsolete CMake library output location (src/libsecp256k1.so) 399b582a5f Split memclear into two versions 4985ac0f89 Merge bitcoin-core/secp256k1#1737: doc: mention ctx requirement for `_ellswift_create` (not secp256k1_context_static) 7ebaa134a7 check-abi: remove support for obsolete CMake library output location (src/libsecp256k1.so) 806de38bfc doc: mention ctx requirement for `_ellswift_create` (not secp256k1_context_static) 03fb60ad2e Merge bitcoin-core/secp256k1#1681: doc: Recommend clang-cl when building on Windows d93380fb35 Merge bitcoin-core/secp256k1#1731: schnorrsig: Securely clear buf containing k or its negation 8113671f80 Merge bitcoin-core/secp256k1#1729: hash: Use size_t instead of int for RFC6979 outlen copy 325d65a8cf Rename and clear var containing k or -k 960ba5f9c6 Use size_t instead of int for RFC6979 outlen copy 737912430d ci: Add more tests for clang-cl 7379a5bed3 doc: Recommend clang-cl when building on Windows f36afb8b3d Merge bitcoin-core/secp256k1#1725: tests: refactor tagged hash verification 5153cf1c91 tests: refactor tagged hash tests d2dcf52091 Merge bitcoin-core/secp256k1#1726: docs: fix broken link to Tromer's cache.pdf paper 489a43d1bf docs: fix broken link to eprint cache.pdf paper d599714147 Merge bitcoin-core/secp256k1#1722: docs: Exclude modules' `bench_impl.h` headers from coverage report 0458def51e doc: Add `--gcov-ignore-parse-errors=all` option to `gcovr` invocations 1aecce5936 doc: Add `--merge-mode-functions=separate` option to `gcovr` invocations 106a7cbf41 doc: Exclude modules' `bench_impl.h` headers from coverage report a9e955d3ea autotools, docs: Adjust help string for `--enable-coverage` option e523e4f90e Merge bitcoin-core/secp256k1#1720: chore(ci): Fix typo in Dockerfile comment 24ba8ff168 chore(ci): Fix typo in Dockerfile comment 74b8068c5d Merge bitcoin-core/secp256k1#1717: test: update wycheproof test vectors c25c3c8a88 test: update wycheproof test vectors 20e3b44746 Merge bitcoin-core/secp256k1#1688: cmake: Avoid contaminating parent project's cache with `BUILD_SHARED_LIBS` 2c076d907a Merge bitcoin-core/secp256k1#1711: tests: update Wycheproof 7b07b22957 cmake: Avoid contaminating parent project's cache with BUILD_SHARED_LIBS 5433648ca0 Fix typos and spellings 9ea54c69b7 tests: update Wycheproof files git-subtree-dir: src/secp256k1 git-subtree-split: d543c0d917a76a201578948701cc30ef336e0fe6
6.7 KiB
Contributing to libsecp256k1
Scope
libsecp256k1 is a library for elliptic curve cryptography on the curve secp256k1, not a general-purpose cryptography library. The library primarily serves the needs of the Bitcoin Core project but provides additional functionality for the benefit of the wider Bitcoin ecosystem.
Adding new functionality or modules
The libsecp256k1 project welcomes contributions in the form of new functionality or modules, provided they are within the project's scope.
It is the responsibility of the contributors to convince the maintainers that the proposed functionality is within the project's scope, high-quality and maintainable. Contributors are recommended to provide the following in addition to the new code:
- Specification: A specification can help significantly in reviewing the new code as it provides documentation and context. It may justify various design decisions, give a motivation and outline security goals. If the specification contains pseudocode, a reference implementation or test vectors, these can be used to compare with the proposed libsecp256k1 code.
- Security Arguments: In addition to a defining the security goals, it should be argued that the new functionality meets these goals. Depending on the nature of the new functionality, a wide range of security arguments are acceptable, ranging from being "obviously secure" to rigorous proofs of security.
- Relevance Arguments: The relevance of the new functionality for the Bitcoin ecosystem should be argued by outlining clear use cases.
These are not the only factors taken into account when considering to add new functionality. The proposed new libsecp256k1 code must be of high quality, including API documentation and tests, as well as featuring a misuse-resistant API design.
We recommend reaching out to other contributors (see Communication Channels) and get feedback before implementing new functionality.
Communication channels
Most communication about libsecp256k1 occurs on the GitHub repository: in issues, pull request or on the discussion board.
Additionally, there is an IRC channel dedicated to libsecp256k1, with biweekly meetings (see channel topic).
The channel is #secp256k1 on Libera Chat.
The easiest way to participate on IRC is with the web client, web.libera.chat.
Chat history logs can be found at https://gnusha.org/secp256k1/.
Contributor workflow & peer review
The Contributor Workflow & Peer Review in libsecp256k1 are similar to Bitcoin Core's workflow and review processes described in its CONTRIBUTING.md.
Coding conventions
In addition, libsecp256k1 tries to maintain the following coding conventions:
- No runtime heap allocation (e.g., no
malloc) unless explicitly requested by the caller (viasecp256k1_context_createorsecp256k1_scratch_space_create, for example). Moreover, it should be possible to use the library without any heap allocations. - The tests should cover all lines and branches of the library (see Test coverage).
- Operations involving secret data should be tested for being constant time with respect to the secrets (see src/ctime_tests.c).
- Local variables containing secret data should be cleared explicitly to try to delete secrets from memory.
- Use
secp256k1_memcmp_varinstead ofmemcmp(see #823). - As a rule of thumb, the default values for configuration options should target standard desktop machines and align with Bitcoin Core's defaults, and the tests should mostly exercise the default configuration (see #1549).
Style conventions
- Commits should be atomic and diffs should be easy to read. For this reason, do not mix any formatting fixes or code moves with actual code changes. Make sure each individual commit is hygienic: that it builds successfully on its own without warnings, errors, regressions, or test failures.
- New code should adhere to the style of existing, in particular surrounding, code. Other than that, we do not enforce strict rules for code formatting.
- The code conforms to C89. Most notably, that means that only
/* ... */comments are allowed (no//line comments). Moreover, any declarations in a{ ... }block (e.g., a function) must appear at the beginning of the block before any statements. When you would like to declare a variable in the middle of a block, you can open a new block:void secp256k_foo(void) { unsigned int x; /* declaration */ int y = 2*x; /* declaration */ x = 17; /* statement */ { int a, b; /* declaration */ a = x + y; /* statement */ secp256k_bar(x, &b); /* statement */ } } - Use
unsigned intinstead of justunsigned. - Use
void *ptrinstead ofvoid* ptr. - Arguments of the publicly-facing API must have a specific order defined in include/secp256k1.h.
- User-facing comment lines in headers should be limited to 80 chars if possible.
- All identifiers in file scope should start with
secp256k1_. - Avoid trailing whitespace.
- Use the constants
EXIT_SUCCESS/EXIT_FAILURE(defined instdlib.h) to indicate program execution status for examples and other binaries.
Tests
Coverage
This library aims to have full coverage of reachable lines and branches.
To create a test coverage report, configure with --enable-coverage (use of GCC is necessary):
$ ./configure --enable-coverage
Run the tests:
$ make check
To create a report, gcovr is recommended, as it includes branch coverage reporting:
$ gcovr --gcov-ignore-parse-errors=all --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --print-summary
To create a HTML report with coloured and annotated source code:
$ mkdir -p coverage
$ gcovr --gcov-ignore-parse-errors=all --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --html --html-details -o coverage/coverage.html
On gcovr >=8.3, --gcov-ignore-parse-errors=all can be replaced with --gcov-suspicious-hits-threshold=140737488355330.
Exhaustive tests
There are tests of several functions in which a small group replaces secp256k1. These tests are exhaustive since they provide all elements and scalars of the small group as input arguments (see src/tests_exhaustive.c).
Benchmarks
See src/bench*.c for examples of benchmarks.