Building on 32-bit systems fail

[steev]

We have kyber in our Kali Linux repositories as a dependency of Merlin and one thing that has come up in our CI is that kyber does not build on 32-bit systems.

You can see our build logs for armel, armhf, and i386 at each of the links; the basic gist is that while it compiles fine on 32-bit, the tests are failing.

--- FAIL: Test_Example_DKG (0.14s)
panic: dkg: cannot process own deal: Error while decoding field {Name:SecShare PkgPath: Type:*share.PriShare Tag: Offset:12 Index:[1] Anonymous:false}: Error while decoding field {Name:I PkgPath: Type:int Tag: Offset:0 Index:[0] Anonymous:false}: detected a 32bit machine, please use either int64 or int32 [recovered]
    panic: dkg: cannot process own deal: Error while decoding field {Name:SecShare PkgPath: Type:*share.PriShare Tag: Offset:12 Index:[1] Anonymous:false}: Error while decoding field {Name:I PkgPath: Type:int Tag: Offset:0 Index:[0] Anonymous:false}: detected a 32bit machine, please use either int64 or int32
goroutine 19 [running]:
testing.tRunner.func1.2({0x821f8e0, 0x88903b0})
    /usr/lib/go-1.21/src/testing/testing.go:1545 +0x2ab
testing.tRunner.func1()
    /usr/lib/go-1.21/src/testing/testing.go:1548 +0x43e
panic({0x821f8e0, 0x88903b0})
    /usr/lib/go-1.21/src/runtime/panic.go:914 +0x1ec
go.dedis.ch/kyber/share/dkg/pedersen.(*DistKeyGenerator).Deals(0x88983c0)
    /builds/kalilinux/packages/golang-go.dedis-kyber/debian/output/source_dir/_build/src/go.dedis.ch/kyber/share/dkg/pedersen/dkg.go:301 +0x346
go.dedis.ch/kyber/examples.Test_Example_DKG(0x88fe0f0)
    /builds/kalilinux/packages/golang-go.dedis-kyber/debian/output/source_dir/_build/src/go.dedis.ch/kyber/examples/dkg_test.go:79 +0x48e
testing.tRunner(0x88fe0f0, 0x8262280)
    /usr/lib/go-1.21/src/testing/testing.go:1595 +0x120
created by testing.(*T).Run in goroutine 1
    /usr/lib/go-1.21/src/testing/testing.go:1648 +0x3dd

This is only one example of the failures, and I'm not sure if I should open one for each individually or not.

[ineiti]

This is related to #492. I added some comments over there where I suggest we could probably change all ints to int64 and it should work.

The actual failing part is here: https://github.com/search?q=repo%3Adedis%2Fprotobuf+32bit&type=code, because if you would use the DKG in a mixed 32/64 bit environment, it will fail. However, following Merlin -> opaque, and looking how it is used there, I don't think it's using the DKG part of kyber at all. So if you can skip the tests, I think you should be safe.

You don't need to open issues for each failure.

@pierluca now that we have an actual failing use-case for #492, shall we try to fix it by putting int64 everywhere?

[steev]

I was trying to skip all of the tests, however, while skipping them, I ran into the genDistSecrets in an init function, and this was causing a segfault; I'm not familiar enough with go, so for now, we just skip all of the tests, and hopefully we can re-enable them once there is a release with the 32bit support :)

[ineiti]

Thanks for the comment. Can you elaborate a bit more, please? You wrote that you skipped all the tests, but then ran into a call to genDistSecrets. But I looked for it and couldn't find it anywhere in the code. Is it a typo? Or is this something in your code?

[steev]

Hi, it's genDistSecret not Secrets, sorry about that!

And, what I attempted to do was t.Skip() the failing tests only, and ran into

panic: Error while decoding field {Name:SecShare PkgPath: Type:*share.PriShare Tag: Offset:12 Index:[1] Anonymous:false}: Error while decoding field {Name:I PkgPath: Type:int Tag: Offset:0 Index:[0] Anonymous:false}: detected a 32bit machine, please use either int64 or int32

goroutine 1 [running]:
go.dedis.ch/kyber/share/dkg/rabin.(*DistKeyGenerator).Deals(0xc63680)
        /<<PKGBUILDDIR>>/_build/src/go.dedis.ch/kyber/share/dkg/rabin/dkg.go:249 +0x218
go.dedis.ch/kyber/sign/dss.genDistSecret()
        /<<PKGBUILDDIR>>/_build/src/go.dedis.ch/kyber/sign/dss/dss_test.go:159 +0x1b4
go.dedis.ch/kyber/sign/dss.init.0()
        /<<PKGBUILDDIR>>/_build/src/go.dedis.ch/kyber/sign/dss/dss_test.go:37 +0x1a0
FAIL    go.dedis.ch/kyber/sign/dss      0.056s

but what i couldn't figure out was which test this was associated with as it was between TestMask and TestEdDSAMarchalling and both of those tests passed.

[ineiti]

but what i couldn't figure out was which test this was associated with as it was between TestMask and TestEdDSAMarchalling and both of those tests passed.

That might be go test which doesn't cleanly separates the output between different tests. I think it tries to run things in parallel, and doesn't always correctly handle output coming from different portions of the system.

Instead of t.Skip, we could add a build constraint to the file: https://pkg.go.dev/cmd/go#hdr-Build_constraints

What architecture are you working on?

[steev]

I work with amd64, arm64, armhf, armel and i386 - so 2 64-bit and 3 32-bit :) I think overall though, maybe the constraints are overkill, and this all goes away when 492 is taken care of, so maybe not focus on this at all?

[ineiti]

If we can fix it with a build constraint it can be solved in v3 of kyber. For #492 we need to update the version to v4, which seems to be somewhat planned for next Spring / Summer. But this is not really confirmed yet :)

So, yes, #492 is the way to go. But it'll take time. Speak up if you're looking for a project...

@pierluca what do you think about adding some build constraints in the dkg directories? We either add explicitly working 64-bit architectures, or exclude 32-bit ones. I have a small preference for excluding, as I believe it's more rare to have 32-bit nowadays, than seeing a new 64-bit popping up where we'd need to add a new entry.

[pierluca]

Sorry for the slow response. Yes, excluding 32-bit architectures would be a reasonable build constraint, but then again, I'm not sure we can call this exactly backwards-compatible... I would also favor (1) documenting this and (2) addressing it as part of #492 and a v4 release, considering this has been an issue for a long time.

[ineiti]

Yes, excluding 32-bit architectures would be a reasonable build constraint, but then again, I'm not sure we can call this exactly backwards-compatible...

It currently panics on 32-bit architectures, so there is no compatibility to be kept :)

I would also favor (1) documenting this and (2) addressing it as part of #492 and a v4 release, considering this has been an issue for a long time.

I thought adding some comments in the package itself. Do you want also comments in the main README.md?

And yes, if there is a v4, this should definitely be addressed at that moment.

@steev do you have some cycles to make a first PR for this? It should contain:

• build constraints for the panicking files
• documentation saying why it's not available on 32-bits
• add a comment to #492, stating that this module can be enabled in 32-bits if the int-sizes are correctly indicated

Else I think later this month I can also do it.