Big endian issues

[mpapierski]

Thanks for great library!

I'm trying to run this on mips machine which has big endian architecture and few tests are failing. I wonder how critical are those problems and should I be worried about correctness of the operations?

$ cross test --target mips-unknown-linux-gnu --all  
warning: depend/check_uint128_t.c: In function 'main':
warning: depend/check_uint128_t.c:5:5: error: unknown type name '__uint128_t'
warning:      __uint128_t var_128;
warning:      ^
warning: depend/check_uint128_t.c:11:16: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning:      if (var_64 == var_128) {
warning:                 ^
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running /target/mips-unknown-linux-gnu/debug/deps/secp256k1-014f3ea81779a358

running 30 tests
test ecdh::tests::ecdh ... ok
test key::test::invalid_secret_key ... ok
test key::test::keypair_slice_round_trip ... ok
test key::test::pubkey_combine ... ok
test key::test::pubkey_equal ... ok
test key::test::pubkey_from_slice ... ok
test key::test::pubkey_hash ... ok
test key::test::skey_from_slice ... ok
test key::test::test_addition ... ok
test key::test::test_debug_output ... ok
test key::test::test_display_output ... ok
test key::test::test_multiplication ... ok
test key::test::test_out_of_range ... ok
test key::test::test_pubkey_from_bad_slice ... ok
test key::test::test_pubkey_serialize ... ok
test tests::bad_recovery ... ok
test tests::capabilities ... ok
test tests::recid_sanity_check ... ok
test tests::sign ... FAILED
test tests::sign_and_verify ... ok
test tests::sign_and_verify_extreme ... ok
test tests::sign_and_verify_fail ... ok
test tests::sign_with_recovery ... ok
test tests::signature_lax_der ... ok
test tests::signature_serialize_roundtrip ... ok
test tests::test_bad_slice ... ok
test tests::test_debug_output ... FAILED
test tests::test_low_s ... ok
test tests::test_recov_id_conversion_between_i32 ... ok
test tests::test_recov_sig_serialize_compact ... ok

failures:

---- tests::sign stdout ----
thread 'tests::sign' panicked at 'assertion failed: `(left == right)`
  left: `Ok(RecoverableSignature(b4abce65aae1f9639c51f2791c60f67c79ae6dcb3224f5eb8e1467b8d8c0ecc0b0ebc3854c7ed95ad741f7a01391a91565bfacaa39c0e9bd01d7ec483f2b125a00))`,
 right: `Ok(RecoverableSignature(092e8898c36dedf4fc439e65af0c1e77921f0ba604772b6f2147741f6673ffad09b68c898d12eed0a39aae06ff2080c4025e709f80120ef852e0ada84c1a971601))`', src/lib.rs:800:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- tests::test_debug_output stdout ----
thread 'tests::test_debug_output' panicked at 'assertion failed: `(left == right)`
  left: `"RecoverableSignature(092e8898c36dedf4fc439e65af0c1e77921f0ba604772b6f2147741f6673ffad09b68c898d12eed0a39aae06ff2080c4025e709f80120ef852e0ada84c1a971601)"`,
 right: `"RecoverableSignature(98882e09f4ed6dc3659e43fc771e0cafa60b1f926f2b77041f744721adff7366898cb609d0ee128d06ae9aa3c48020ff9f705e02f80e1280a8ade05216971a4c01)"`', src/lib.rs:986:9


failures:
    tests::sign
    tests::test_debug_output

test result: FAILED. 28 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--lib'

Thanks!

[apoelstra]

Very cool! This may actually be an upstream issue. Let me investigate.

It looks like the recoverable signatures module is not going to work for you until this bug is fixed -- probably not a big deal, since recoverable signatures are somewhat niche. But definitely a serious bug in the library.

[apoelstra]

Ok, on further investigation I actually think this is pretty minor. There are two bugs, I'm pretty sure

• The RecoverableSignature::Debug impl is wrong and not endian-consistent. This is a minor bug, since this output is expected to be unreliable/not machine parseable anyway. Use the serialize* methods if you want to communicate across processes.
• The RFC6979 implementation is not endian-consistent, because in build.rs we don't compile the library with the right C defines. I can't think of a context in which this would matter at all, though it should be fixed.

Thanks very much for testing with a BE machine!

[mpapierski]

Thanks for quick reply @apoelstra. I did some research regarding your 2nd point. Turns out it makes difference if we define WORDS_BIGENDIAN. With WORDS_BIGENDIAN defined and compiled for mips I got tests::sign test case to succeed. I'm figuring out a way to detect BE from within build.rs. I agree testing Debug output isn't critical for byte order.

[apoelstra]

Partially fixed in #64. Leaving this open until we fix the Debug issue as well.

[mpapierski]

@apoelstra any chance this MIPS fix is part of 0.11.1? I noticed 0.11.1 exists on crates.io, but doesn't exist as a git tag.

[apoelstra]

Oops, I just forgot the push the 0.11.1 tag. Unfortunately not. I can release 0.11.2 this week if you want this fix though.

[mpapierski]

@apoelstra 0.11.2 release will do. I'll use master branch until then. Thanks.

[apoelstra]

Published

[apoelstra]

Added fix for Debug issue in #60

[apoelstra]

Closed by #60