[Feature Request] Make stored transaction error not used for hash calculation

[areshand]

🚀 Feature Request

Motivation

Currenlty, the transaction execution error code is stored in transaction output, which is used in hash calculation.

This is not ideal as a 1. change in error code can lead to a different hashcode and 2. no need to store detailed error code of failed transactions for a long time.

We should do this before the main net

Pitch

Describe the solution you'd like

1. create a new CF in storage to store the transaction errors, which is not used for hash calculation
2. the CF don't need to store data for a long time than 1 day, which is sufficient for most users to read the status to understand why their txn executed but failed.

[areshand]

cc: @msmouse @davidiw

[sitalkedia]

@areshand - Can you clarify which schema in DB you are referring to? Is it the execution status stored in TransctionInfo https://github.com/aptos-labs/aptos-core/blob/main/types/src/transaction/mod.rs#L1014 ?

[lightmark]

I don't understand why this service should be in our db but not as a feature in a 3rd-party service like AptosScan, especially the service data is ephemeral and used for user query purpose only. According to error code change, I believe it is also a breaking change to move vm so even we don't have that it doesn't help the upgrade of move VM.

[areshand]

@areshand - Can you clarify which schema in DB you are referring to? Is it the execution status stored in TransctionInfo https://github.com/aptos-labs/aptos-core/blob/main/types/src/transaction/mod.rs#L1014 ?

Yes, it is the execution status.

[areshand]

eral and used for user query purpose only. According to error code change, I believe it is also a breaking change to move vm so even we don't have that it doesn't help the upgrade of move VM.

I don't understand why this service should be in our db but not as a feature in a 3rd-party service like AptosScan, especially the service data is ephemeral and used for user query purpose only. According to error code change, I believe it is also a breaking change to move vm so even we don't have that it doesn't help the upgrade of move VM.

1. we want users to see the detail error status code when their txn failed.
2. user won't see this error when submitting the txn since errorcode is generated after executing the txn
3. so we need to store the error code somewhere when user query the status of their txn and they see the error code. However, we probably don't want to store it longer (eg more than 1 day)

which place do you recommend to store the error code ?

[areshand]

As recommended by @sitalkedia an alternative is simply defining a custom hash function for TxnInfo should do I believe. I am not sure introducing a new CF for execution status is needed for this.

[lightmark]

1. we want users to see the detail error status code when their txn failed.
2. user won't see this error when submitting the txn since errorcode is generated after executing the txn
3. so we need to store the error code somewhere when user query the status of their txn and they see the error code. However, we probably don't want to store it longer (eg more than 1 day)

which place do you recommend to store the error code ? 3 is critical, if you want it forever I think we should keep it on chain as something like event. If not, then we should put it somewhere such as metadata/indexDB as a separate db and the ecosystem ppl are in charge of developing and maintaining it...

[areshand]

@msmouse 's take _K. My take:

1. a lean / stable version of the error code (the same thing before the change) is part of the TxnInfo, not the more debugging oriented info Bo has been trying to add (Let's restore the TxnInfo definition to its previous version).
2. We store the new error info (potentially changing from time to time for the same kind of error but ran by different versions of the VM / Adaptor) as a separate CF for now, even if it's as small as a Option . And that, along with other non-essential stuff, like txn_by_hash , need to go to a separate store / embedded indexer in the long run.
3. Well, now that we store it separately, it has not to be only a u64.. we can output richer info for an aborted transaction._

[github-actions[bot]]

This issue is stale because it has been open 45 days with no activity. Remove the stale label or comment - otherwise this will be closed in 15 days.

[msmouse]

This keeps coming back..

@runtian-zhou

[wrwg]

Is it possible? Would be great.