Inconsistent cadence JSON encoding output among SDKs

[lmcmz]

Problem

When we try to use json encode & decode to sign a transaction as Payer, we found there is an extra byte in each argument after RLP encoding. 0a in hex or 10 in byte represent new line.

Currently, lilico wallet wanna help user to pay their gas fee. Hence, we are using fcl.js to construct the transaction and signed the payload by user. Then it send to our backend which using flow-go-sdk to reconstruct the transaction from the json request.

When we try to sign the transaction as payer and send to the full transaction to the chain, we always got signature error. Then we found the same transaction encode output is different among two official SDK.

I think it should be consistent when encoding the same transaction among SDKs.

Steps to Reproduce

1. Run a simple example

package main
import (
    "fmt"
    "github.com/onflow/cadence"
    c_json "github.com/onflow/cadence/encoding/json"
)

func main() {
    test := cadence.String("foo")
    bytes, _ := c_json.Encode(test)
    fmt.Println(bytes)
    fmt.Println(string(bytes))
}

2. Output

[123 34 116 121 112 101 34 58 34 83 116 114 105 110 103 34 44 34 118 97 108 117 101 34 58 34 102 111 111 34 125 10]
{"type":"String","value":"foo"}

There is an extra 10 byte at the end, which is a new line character, I believe it shouldn't be here.

[turbolent]

Thank you for reporting this.

There is no canonical JSON encoding at the moment, i.e. there are many different ways of encoding the same JSON data. For example, whitespace, property order, etc.

There are currently no guarantees two SDKs will generate the same output for encoding Cadence values, and this is also supported/accepted by the node software.

[lmcmz]

I think It should be consistent at least among those flow official SDK, otherwise it just will cost hazard to developer if people like us wanna implement multi-sign transaction across different platform or env. You know just an Incorrect byte will cause the whole transaction failed.

[lmcmz]

Update description with more context.

[turbolent]

Agreed. We are working on a new external value encoding format (#1651), which will have several advantages, including having a canonical format, being more compact, etc.

[psiemens]

Thanks for reporting this @lmcmz.

I think it probably makes sense to make the JSON encoding consistent between the SDKs. But that's really only a band-aid fix IMO, given that the JSON spec itself makes no guarantees about the consistency. As Bastian mentioned, the new value encoding format should provide a better long term solution.

In the immediate term, the best way to mitigate this is to avoid decoding and re-encoding the JSON after the initial signature. Could you share an example of how you are reconstructing the transaction in FCL? That will help me pinpoint where changes could be made (either to your code or FCL).

[lmcmz]

Hi @psiemens

Here is the example code :

For the fcl js part, that's how we send the transaction request by using the voucher in signable project.

    const tx = signable.voucher;
    const message = signable.message;
    const envelope = await api.signPayer({tx, message});

And here is the code for how we reconstruct the transaction in go.

func ArgAsCadence(a forms.Argument) (cadence.Value, error) {
	c, ok := a.(cadence.Value)
	if ok {
		return c, nil
	}
	// Convert to json bytes so we can use cadence's own encoding library
	j, err := json.Marshal(a)
	if err != nil {
		return cadence.Void{}, err
	}
	// Use cadence's own encoding library
	c, err = c_json.Decode(j)

	if err != nil {
		return cadence.Void{}, err
	}
	return c, nil
}

func PayerSign(raw forms.Transaction, network string, message forms.Message) (*flow.Transaction, error) {

	payerAddr, privateKeyHex, keyIndex := FlowService(network)
	sequence := uint64(raw.ProposalKeys.SequenceNumber)

	privateKey, decodeErr := crypto.DecodePrivateKeyHex(crypto.ECDSA_P256, privateKeyHex)
	if decodeErr != nil {
		log.Println(decodeErr)
	}
	payerHexAddr := flow.HexToAddress(payerAddr)
	proposerHexAddr := flow.HexToAddress(raw.ProposalKeys.Address)

	signer := crypto.NewInMemorySigner(privateKey, crypto.SHA2_256)
	refBlock := flow.HexToID(raw.ReferenceBlockId)
	tx := flow.NewTransaction().
		SetScript([]byte(raw.Script)).
		SetGasLimit(raw.GasLimit).
		SetProposalKey(proposerHexAddr, raw.ProposalKeys.KeyIndex, sequence).
		SetPayer(payerHexAddr).
		SetReferenceBlockID(refBlock)

	if payerHexAddr == proposerHexAddr {
		return tx, nil
	}

	for _, v := range raw.Authorizers {
		authHexAddr := flow.HexToAddress(v)
		if payerHexAddr == authHexAddr {
			return tx, nil
		}
		tx.AddAuthorizer(authHexAddr)
	}

	for _, v := range raw.Arguments {
		cv, err := ArgAsCadence(v)
		if err != nil {
			log.Println(err)
		}

		err = tx.AddArgument(cv)
		if err != nil {
			log.Println(err)
		}
	}

	for _, v := range raw.PayloadSignature {
		hexString, _ := hex.DecodeString(v.Signature)
		byteString := []byte(hexString)
		tx.AddPayloadSignature(flow.HexToAddress(v.Address), v.KeyIndex, byteString)
	}

	trimmingArg := tx.Arguments
	for i, v := range trimmingArg {
		v = bytes.TrimSuffix(v, []byte("\n"))
		trimmingArg[i] = v
	}

	tx.Arguments = trimmingArg
	envMessage := tx.EnvelopeMessage()
	envMessage = append(flow.TransactionDomainTag[:], envMessage...)

	err := tx.SignEnvelope(flow.HexToAddress(payerAddr), keyIndex, signer)
	if err != nil {
		return tx, err
	}
	return tx, nil
}

[lmcmz]

Hi @psiemens @sideninja @turbolent

Unfortunately, I found some other issue when encode the UFix64 or Fix64, the precision is not aligned 🙁. ( I guess I need move lilico payer API from go server to js server to keep it working for now. )

It should relate with this line: https://github.com/onflow/cadence/blob/7bf6d8d5c3f6ea4368ecfb22411748239fa1a807/encoding/json/encode.go#L904

fcl.js

{"type":"UFix64","value":"0.0"}

flow go sdk

{"type":"UFix64","value":"0.00000000"}

Example

package main
import (
    "fmt"
    "github.com/onflow/cadence"
    c_json "github.com/onflow/cadence/encoding/json"
)

func main() {
    test := cadence.UFix64(0.0)
    bytes, _ := c_json.Encode(test)
    fmt.Println(bytes)
    fmt.Println(string(bytes))
}

Result

[123 34 116 121 112 101 34 58 34 85 70 105 120 54 52 34 44 34 118 97 108 117 101 34 58 34 48 46 48 48 48 48 48 48 48 48 34 125 10] {"type":"UFix64","value":"0.00000000"}

[devbugging]

I have made PRs for both fixes, the fix for stripping and extra new line is done in Go SDK https://github.com/onflow/flow-go-sdk/pull/292 The PR for extending decimal places to 8 is done on FCL https://github.com/onflow/fcl-js/pull/1234