Duplicate case in switch (compile error) when building without cgo

[bartolomej]

Problem Definition

I'm trying to build flow-emulator for JS/WASM target, but I'm running into this error when building without cgo:

# wasm/main.go is just my entry point that imports flow-go and flow-emulator
$ CGO_ENABLED=0 GOOS=js GOARCH=wasm go build -tags=no_cgo -o flow-emulator.wasm wasm/main.go
# github.com/onflow/flow-go/module/signature
../../go/pkg/mod/github.com/!the-!k-!r-!o-!k/flow-go@v0.34.0-crescendo-preview.9.0.20240508085818-dd583cbc9a7b/module/signature/type_encoder.go:68:7: duplicate case 2 * SigLen (constant 0 of type int) in expression switch
        ../../go/pkg/mod/github.com/!the-!k-!r-!o-!k/flow-go@v0.34.0-crescendo-preview.9.0.20240508085818-dd583cbc9a7b/module/signature/type_encoder.go:66:7: previous case

Looks like SigLen is 0 when building without cgo - I believe it's because C source isn't included/compiled in the build, so I'm guessing it just defaults to 0?

flow-go/module/signature/type_encoder.go

Lines 65 to 70 in e4bdb7e

	switch sigLength {
	case SigLen:
	return sigData, nil, nil
	case 2 * SigLen:
	return sigData[:SigLen], sigData[SigLen:], nil
	}

Proposed Solution

Is there a simple fix we can do to take into account this edge case? Maybe using if/else-if statement instead of switch doesn't cause the same compiler error or something like that?

Definition of Done

flow-go compiles successfully with command such as:

CGO_ENABLED=0 go build -tags=no_cgo cmd/execution/main.go

[turbolent]

Seems odd that SigLen ends up being 0 🤔

cc @tarakby

[tarakby]

You're right @bartolomej, building the crypto library without cgo disables everything related to BLS signature, and the length that is normally read from the C code is set to zero.

I only added and tested the no_cgo mode to the crypto repo. The idea is, if cgo is disabled, nothing related to BLS should be expected to work properly, the algorithm is simply not supported (functions would panic and constants values do not make sense).
I believe the flow-go repo always requires BLS in some way, and I did not test building it with no_cgo (the readme mentions cgo is required).
Same for the flow-emulator, I am not familiar with the repo and to what extent it relies on flow-go or BLS. If it is expected that flow-emulator builds without cgo, then some functions in the repo may need to be adjusted (for example why the call you pointed out to DecodeDoubleSig is needed while BLS isn't). I think this is a problem for the flow-emulator repo to solve.

[tarakby]

Maybe @jribbink knows more about the flow-emulator build for wasm?

[bartolomej]

@tarakby I guess I just assumed that flow-go is also built in a way that it doesn't require cgo. Didn't realise it says that in the README.

I believe the flow-go repo always requires BLS in some way...

After quickly looking into the codebase, it seems like this is true. One example is the PublicKey.verifyPoP function exposed to Cadence.

So it looks like we can't build a WASM emulator that fully matches the non-WASM emulator in feature completeness without cgo enabled. But it doesn't work to enable CGO when building for wasm/js target, since that's just not yet supported by Go natively.

Related discussion: https://stackoverflow.com/questions/54946214/cgo-library-build-to-js-wasm-file

The above discussion does however mention there seems to be a way to use cgo and also compile C code to wasm (or at least that Qt somehow solved that limitation), but that sounds pretty hacky and complicated, so not sure if it's worth digging into that.

So in summary, I believe we could:

• hope that WASM flow-emulator still works good enough that it would be useful, even without BLS (and warn users of some features not working)
• see if there is some way to import C code to Go as a wasm binary instead of a native OS executable (may be a difficult task)

For context, I'm trying to build a PoC of the new fully browser-based playground. We also made this grant proposal: onflow/developer-grants#260

[tarakby]

Thanks for the context!

• flow-go` is the Flow distributed protocol repo, and the protocol heavily relies on BLS, so I don't think it will be possible to make it build without BLS (and therefore without cgo). I know your message doesn't challenge this, but I'm recalling it before the next points :)
• the flow-emulator repo however, does not implement the distributed protocol (or implements a dummy simulated version of the distributed protocol), so it can make sense to build it without BLS (and cgo). Now that the crypto library has a no-cgo mode, this should be even easier. Any call to a BLS-related function or constant should be replaceable in flow-emulator, using the Go build tags(example, we can also use the native tag wasm).
• From your summary points, the first point seems more reasonable. A wasm-flow-emulator would exclude the Cadence BLS functions. There are still other cryptographic algorithms to use on Cadence so I don't think any developer would miss BLS IMO.
• I think we can close this issue and open an issue on the flow-emulator repo to make it wasm buildable (@turbolent or @jribbink feel free to suggest something else)

[bartolomej]

@tarakby Upon discussing this with @bluesign, looking at the code more closely, and reading your response I think I agree.

So in summary:

• Since the flow-emulator doesn't depend on any protocol code in flow-go, BLS won't need to be used for things like consensus
• I think the only place where BLS is used within the emulator is in the Cadence crypto library (exposed in fvm/environment/crypto_library.go).
  • But this also shouldn't be a big issue, since we can just disallow usage of BLS crypto algorithms (for hashing and signing) when doing things like creating account keys (in flow-emulator / flow-cli / UI).
  • In case someone does use BLS functions in their Cadence code, we could also detect that on our end and report an error. Probably wouldn't be a deal breaker for most people as you mentioned.

[bartolomej]

@tarakby @turbolent However for building flow-emulator without cgo, this "duplicate case in switch" compiler error still prevent us from having a successful build. But since that is our concern, I'm thinking of just using my forked repo (where I've fixed the issue already) instead of this one, which would come at a slight cost of having to keep that up-to-date. Do you agree this is the way to go?

[tarakby]

just disallow usage of BLS crypto algorithms (for hashing and signing) when doing things like creating account keys (in flow-emulator / flow-cli / UI).

The good news is that BLS can't be used as an account key algorithm, only ECDSA can. BLS isn't used by CLI, and can only be used by the flow-emulator if used in a Cadence contract (a niche use-case as you mentioned).

I tried out the fix and noticed it's enough to make flow-emulator build and pass tests. I initially thought the emulator requires multiple fixes and the one you pointed is only the first error. I opened this PR in flow-go, feel free to open a PR on flow-emulator to import the new flow-go version.
That would avoid you relying on a fork. In general, feel free to open PRs against the main repos, they will get accepted as long as they don't change the main case behaviour.