Observable Transaction Status

[LayneHaber]

Summary

Ability to tell if transaction is fulfilled or cancelled onchain is not currently possible, instead the TransactionManager will only indicate if a transaction is fully completed. This leads to some annoying sanitation checks needed in the router. Instead, the status should be hashed with the transaction when stored onchain to disambiguate between the two end states.

Motivation

In v0, the TransactionManager stores a hash of the VariantTransactionData onchain. When the transaction is Prepared, the true preparedBlockNumber is stored. When the transaction is either Fulfilled or Cancelled the preparedBlockNumber is set to 0 and then stored onchain as part of the VariantTransactionData. This accomplishes the onchain task of determining if a transaction can be completed, but makes offchain validation of transaction actions difficult. Since there is no indication of how a transaction was completed, special logic has to be created to determine the status of a completed transaction.

Proposed Solution

Instead, the status should be stored onchain. This is especially important as nomad can complicate message tracking. Additionally, the nomad token bridge is restricted in the information it will pass through.

Store the status as part of the FulfilledTransaction:

enum TransactionStatus {
    Fulfilled,
    Reconciled,
    Processed
}

struct FulfilledTransaction {
    ...
    TransactionStatus status;
  }

Should also include a getTransactionStatus(bytes32 transactionId) helper function.

Test Cases

The following should be implemented:

it("should update the status to Fulfilled after a transaction is fulfilled", () => { ... })
it("should update the status to Reconciled after a transaction is reconciled", () => { ... })
it("should update the status to Processed after a transaction is processed", () => { ... })
it("should return the correct status via getter", () => { ... })

NOTE The above assertions can also be included in the assertions for normal happy case tests

Outstanding Questions

[sanchaymittal]

I don't we need empty status.

Empty should be when variantTransactionData is "0x" for an Invariant digest.

[LayneHaber]

I think there is a solidity quirk around this (specifically enums get translated to numbers, so a transaction would by default be in the Prepared status without this).

We can add the check around whether the digest is empty, but that's returning the requirement of some additional knowledge outside of just Status.

[sanchaymittal]

Agreed that enum default value is index 0, but we aren't mapping the transactionId/transferId to enum. Only using it as a param in hashing the variant, so it should be fine. But if it doesn't really add much gas then it's fine.

[LayneHaber]

Good point! Was thinking we could have a getTransactionStatus(TransactionData tx) {} function so having the status be accurate to what is happening would be useful. If the concern is gas costs then I think we can flag it and test it out

[sanchaymittal]

sounds good!

[LayneHaber]

Updated onchain, we no longer need an Empty status because at the time of reconcile Empty and Fulfill would be treated the same.

[LayneHaber]

No longer relevant, subgraph + cache can accomplish this