BlockstreamResearch / simplicity

Simplicity is a blockchain programming language designed as an alternative to Bitcoin script.
MIT License
305 stars 45 forks source link

Remove secp256k1 32-bit specific code. #256

Closed roconnor-blockstream closed 1 month ago

roconnor-blockstream commented 2 months ago

This all but eliminate any concerns about different representations of secp256k1 jet results between 32 and 64 bit platforms (which could lead to consensus failures).

This can be tested using, for example,

nix-build -A c --arg nixpkgs '(import <nixpkgs> {}).pkgsi686Linux'
roconnor-blockstream commented 2 months ago

Note, we can still build on 32-bit platforms. It just uses the struct-based implementation of wide multiplication with the "64-bit algorithms" rather than the specialized 32-bit algorithms.

apoelstra commented 2 months ago

With my copy of nixpkgs if I try to build with pkgsi686Linux I get errors of the form

secp256k1/int128_native.h:16:9: error: unknown type name 'uint128_t'
   16 | typedef uint128_t secp256k1_uint128;
      |         ^~~~~~~~~
roconnor-blockstream commented 2 months ago

@apoelstra I think you are going to have to add debugging pragmas to https://github.com/BlockstreamResearch/simplicity/blob/1ee7c32576404d56f3164260f66342eff69a4e3c/C/secp256k1/util.h#L89-L120 in order to figure out why that is getting run on pkgsi686Linux. It should only be used if defined(UINT128_MAX) || defined(__SIZEOF_INT128__).

apoelstra commented 2 months ago

Oh, I see. Probably I am forcing these to be on as part of my CI setup.

apoelstra commented 2 months ago

I assume it's because I have wideMultiply set to "int128". I will update my CI setup to not do that on 32-bit builds.

roconnor-blockstream commented 2 months ago

Yeah, if you are testing with USE_FORCE_WIDEMUL_INT128, then that is expected to fail on 32 bit platforms.

apoelstra commented 1 month ago

Also (new in this PR) SECP256K1_WIDEMUL_INT64 is disabled for all platforms, not just 32-bit ones.

I would suggest removing the int64 option from the nixfile or at least erroring out on it early. (Can be done in a followup.)