Transaction ID mismatch between SDK and flow model

[peterargue]

Problem

Someone on discord reported an issue where the flow-cli shows a different transaction ID from the one requested. e.g.

$ flow transactions get 38559cfac11eba732f3de2420eac511393e223bc92b0c52f75b6604098e4181a --network=mainnet    

Block ID        7db0f9d4867e1c895f83a5e03e932cebfd6e3daeed53b8108d9c75e097c58784
Block Height    56134268
Status          ✅ SEALED
ID              06e254293add97d9a9519eac42a30c8450ff7f51556392901462f5476ca06a18
Payer           8da51d7dff4b3151
Authorizers     [8da51d7dff4b3151]

Proposal Key:
    Address     8da51d7dff4b3151
    Index       0
    Sequence    9633

No Payload Signatures

Envelope Signature 0: 8da51d7dff4b3151
Signatures (minimized, use --include signatures)

Events:          
    Index       0
    Type        A.1654653399040a61.FlowToken.TokensWithdrawn
    Tx ID       38559cfac11eba732f3de2420eac511393e223bc92b0c52f75b6604098e4181a
    Values
                - amount (UFix64): 73.09790833 
                - from (Address?): 0x8da51d7dff4b3151 
...

Note that the requested TX was 38559cfac11eba732f3de2420eac511393e223bc92b0c52f75b6604098e4181a, and the ID included in the response was 06e254293add97d9a9519eac42a30c8450ff7f51556392901462f5476ca06a18. Also note that the ID in the events correctly match the request.

Looking at the flow-cli code, the ID field is populated using the tx.ID() method on the transaction returned using the SDK.

The ID() generates a hash of the datastructure, so this most likely means that the serialized representation of the transaction produced by the sdk is different from the flow model used to produce the original.

Steps to Reproduce

I created this repo script: https://gist.github.com/peterargue/da744c9e7c4bddbfee4d89240a3f4244

It requests the same transaction from an AN using the sdk and grpc directly. Calling ID() on the sdk version produces 06e254293add97d9a9519eac42a30c8450ff7f51556392901462f5476ca06a18, but using the flow model, it returns 38559cfac11eba732f3de2420eac511393e223bc92b0c52f75b6604098e4181a.

[bluesign]

yeah this is very old issue ( related to cadence json in args )there was a fix in flow-go-sdk to match fcl-js behaviour. ( while encoding arguments, extra new line or not issue )

related: onflow/flow-cli#679

[peterargue]

I think this is a little different. The SDK uses grpc under the covers (I think), so in my reproduction, the SDK and my script received identical responses from the API. The only difference was the logic that converts from protobuf to a go struct.

[bluesign]

@peterargue
here is the place I mentioned:

flow-go-sdk/transaction.go

Lines 372 to 376 in 26fbdab

	// note(sideninja): This is a temporary workaround until cadence defines canonical format addressing the issue https://github.com/onflow/flow-go-sdk/issues/286
	for i, arg := range t.Arguments {
	if arg[len(arg)-1] == byte(10) { // extra new line character
	t.Arguments[i] = arg[:len(arg)-1]
	}

Here is go json encoder part:
https://github.com/rsc/xstd/blob/55f693673a79603c776ccb0bf91dd0c985148051/go1.14.9/encoding/json/stream.go#L212-L218

Basically go is adding extra \n when encoding Cadence.Value, fcl-js is not. There was some issue long ago, when some server side signing tx ID was not matching, so flow-go-sdk added this hack. (#292)

Now removing this hacks for sure breaks someones workflow, so canonical encoding is waited (I suppose). ( in the mean time it can be fixed on cli side, but just passing txid (like: onflow/flow-cli#679 ) )

[peterargue]

Yea, that's it. I tried my script with that code disabled and get the same tx ID. here's the args as hex

$ go run cmd/tx_result/main.go
SDK TxID:        06e254293add97d9a9519eac42a30c8450ff7f51556392901462f5476ca06a18
SDK Result TxID: 38559cfac11eba732f3de2420eac511393e223bc92b0c52f75b6604098e4181a
gRPC TxID:       38559cfac11eba732f3de2420eac511393e223bc92b0c52f75b6604098e4181a
SDK Args:
  0: 7b2276616c7565223a22307832323234643462383331663534613765222c2274797065223a2241646472657373227d
  1: 7b2276616c7565223a2237332e3039373930383333222c2274797065223a22554669783634227d
gRPC Args:
  0: 7b2276616c7565223a22307832323234643462383331663534613765222c2274797065223a2241646472657373227d0a
  1: 7b2276616c7565223a2237332e3039373930383333222c2274797065223a22554669783634227d0a

I'm guessing the the user that submitted the problem tx was just using an older version of the sdk that didn't include this change.

I think the problem is just that the SDK is using this hack for both new and already submitted tx. This makes sense for tx it submits, but for any received via the API, the hack should be skipped.

[devbugging]

@peterargue for existing old Tx you are right, the problem is there's no good way of knowing in that SDK implementation where the payload came from unless breaking a lot of stuff. I believe the new CCF format will address this better and hopefully greatly reduce any kind of similar issues. If you see any action we can take let me know.

[devbugging]

Related to: #377