[Malleability] TransactionResult

[UlyanaAndrukhiv]

TransactionResult Malleability

flow-go/model/flow/transaction_result.go

Lines 7 to 17 in edf27b0

	// TransactionResult contains the artifacts generated after executing a Cadence transaction.
	type TransactionResult struct {
	// TransactionID is the ID of the transaction this error was emitted from.
	TransactionID Identifier
	// ErrorMessage contains the error message of any error that may have occurred when the transaction was executed
	ErrorMessage string
	// Computation used
	ComputationUsed uint64
	// Memory used (estimation)
	MemoryUsed uint64
	}

The TransactionResult struct is used as a flow.Entity to pretty-print the object when queried from the command line. In the transaction-results command, TransactionResult objects are retrieved and displayed using the PrettyPrintEntity() function. Therefore, we need to ensure that the identifier used for this object is fully representative of all fields, preventing any malleability issues.

flow-go/cmd/util/cmd/common/print.go

Lines 12 to 16 in edf27b0

	// PrettyPrintEntity pretty print a flow entity
	func PrettyPrintEntity(entity flow.Entity) {
	log.Info().Msgf("entity id: %v", entity.ID())
	PrettyPrint(entity)
	}

The current TransactionResult implementation uses the ID() method to return the TransactionID as the unique identifier:

flow-go/model/flow/transaction_result.go

Lines 24 to 27 in edf27b0

	// ID returns a canonical identifier that is guaranteed to be unique.
	func (t TransactionResult) ID() Identifier {
	return t.TransactionID
	}

This method assumes that TransactionID is sufficient to uniquely identify a TransactionResult. While this approach is simple, it introduces potential malleability concerns. The identifier computation currently only considers the TransactionID field, leaving out important fields like:

• ErrorMessage,
• ComputationUsed,
• MemoryUsed.

This omission means that two TransactionResult instances with the same TransactionID but differing values for these fields would produce the same identifier, which is incorrect.

Proposed Solution

Key Changes

1. Update ID() : TransactionResultis used as flow.Entity to pretty print object
   To address these concerns, the ID() method will be updated to compute the identifier based on the entire TransactionResult struct using the MakeID() function:

func (t *TransactionResult) ID() Identifier {
    return MakeID(t)
}

By encoding all fields, this approach guarantees that the resulting ID() is malleability-resistant. Changes to any field in TransactionResult will produce a different identifier.

All fields will be encoded in a way that guarantees consistency:

• Primitive fields: fields like TransactionID, ComputationUsed, and MemoryUsed are encoded directly as raw bytes using RLP.
• String field: like ErrorMessage are also RLP-encoded to ensure consistency across different string values (encoding rlp rules).

2. Remove unused function: During the revision process, it was identified that the function Checksum() is unused.

Definition of Done

1. The TransactionResult.ID() method has been updated using MakeID(). The identifier computation has been verified to use RLP encoding for all fields.
2. Checksum() function has been removed.
3. Unit tests have been updated to validate the new behavior, ensuring identifiers change as expected when data is modified.
4. Documentation and comments have been updated to reflect the changes and clarify the purpose of the ID() method.