Skip to content

Conversation

@bitmarkcc
Copy link

This pull request fixes some compilation errors present in the current tip of the master branch, fixes a bug that affects headers-first sync compatibility, as well as numerous other improvements, which can be read in the release notes for 0.9.7.5 and 0.9.7.6.

This pull request prepares us for the next stage (v27.x+), i.e. the pull request by @jimmypound.

dbkeys and others added 30 commits February 23, 2021 06:52
Compiles under TLS/SSL libraries in Ubuntu 20 "Focal Fossa"
Merge SSL branch (changed to compile under new SSL/TLS) back into master
…eys-ssl-only

Merge ssl-only branch back into master branch;
This commit will be tagged as Release v0.9.7.4
Development of TLS / SSL compatibility changes for latest Linux OS
is merged back into master branch
This reverts commit 7a6d96d.

Conflicts:
	src/key.h
	src/rpcprotocol.h
Conflicts:
	src/rpcserver.h
Conflicts:
	src/rpcserver.h
Coin Runner and others added 20 commits April 28, 2024 16:16
…ihash and cryptonight blocks when "getheaders" are requested.
…nversion for the current and peak hashrate RPC calculations. Use a custom version of the bn.h header, with BN_ULONG set to uint64_t when the platform is 32 bit.
…ing factor (as is done on 64 bit machines, and should be done on 32 bit machines as well). Restore the openssl/bn.h includes.
…t, then use the max uint64_t as the uint64_t value. This is consistent with openssl's bignum
@jimmypound
Copy link

Overall looks good to me. The only concern is using BoostBigNum instead of CBigNum. If there is any difference in calculation or rounding in these two types we might get a chain split. Similarly, I tried to use arith_uint256 for v.27 updated, but there were small difference in rounding which prevent chain sync.

I'll try to do a basic testing of this version when I find some time.

@bitmarkcc
Copy link
Author

Thanks for looking...The BigNum SSF calculation has an overflow issue that we thought about fixing earlier and it behaves differently on 32 bit vs 64 bit machines. As long as most miners are using 64 bit machines, there is no problem, but I want to make sure everything is compatible, so that's why I did that.

The overflow behavior can be looked at positively also, since it causes the subsidy to 'jump around' when hashrate is close to peak hashrate (>97.6%), which can be good in preventing miners from sitting close to the peak and getting full reward without ever crossing the peak.

By the way we tried contacting you on the email you put in your git commits, but no response. If you don't want to chat, that's ok, but we can if you want.

@jimmypound
Copy link

Thanks for looking...The BigNum SSF calculation has an overflow issue that we thought about fixing earlier and it behaves differently on 32 bit vs 64 bit machines. As long as most miners are using 64 bit machines, there is no problem, but I want to make sure everything is compatible, so that's why I did that.

It makes sense. I did a basic test and everything looks good.

By the way we tried contacting you on the email you put in your git commits, but no response. If you don't want to chat, that's ok, but we can if you want.

Sorry, I missed it. I just replied.

@melvincarvalho melvincarvalho requested a review from Copilot January 8, 2026 15:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request updates the secp256k1 library to version 0.9.7.6, addressing compilation errors and fixing a headers-first sync compatibility bug. The changes include new cryptographic implementation files, build system configurations, and mathematical verification scripts.

Key changes:

  • Addition of complete secp256k1 library implementation files for field arithmetic, elliptic curve operations, and ECDSA
  • Build system support via CMake and pkg-config
  • Mathematical verification scripts using Sage for proving correctness of group operations
  • Assembly optimizations for ARM32 architecture

Reviewed changes

Copilot reviewed 141 out of 224 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/secp256k1/src/field_10x26.h Defines field element representation using 10 uint32_t limbs in base 2^26
src/secp256k1/src/field.h Generic interface for secp256k1 field element operations
src/secp256k1/src/ecmult_impl.h Implementation of elliptic curve multiplication algorithms
src/secp256k1/src/ecmult_gen_impl.h Generator point multiplication with blinding for timing attack protection
src/secp256k1/src/ecdsa_impl.h ECDSA signature operations implementation
src/secp256k1/src/bench*.c/h Benchmarking infrastructure and tests
src/secp256k1/src/CMakeLists.txt CMake build configuration
src/secp256k1/sage/*.sage Mathematical proofs for cryptographic operations
src/secp256k1/libsecp256k1.pc.in pkg-config template
src/secp256k1/include/secp256k1_ecdh.h ECDH module header

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

} secp256k1_fe_storage;

#define SECP256K1_FE_STORAGE_CONST(d7, d6, d5, d4, d3, d2, d1, d0) {{ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }}
#define SECP256K1_FE_STORAGE_CONST_GET(d) d.n[7], d.n[6], d.n[5], d.n[4],d.n[3], d.n[2], d.n[1], d.n[0]
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after comma between 'd.n[4]' and 'd.n[3]'. Add a space for consistency with the rest of the macro.

Copilot uses AI. Check for mistakes.
return 0;
}
if (rlen != (size_t)(sigend - sig)) {
/* Tuple exceeds bounds or garage after tuple. */
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'garage' to 'garbage'.

Suggested change
/* Tuple exceeds bounds or garage after tuple. */
/* Tuple exceeds bounds or garbage after tuple. */

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants