[Access] Add endpoints to execution nodes to support tx result err msgs

[AndriiDiachuk]

Description

onflow/flow-go#4754

This pull request introduces a new GRPC API call which gets the error messages of all failed transactions in the block ordered by transaction index.

[peterargue]

Thanks for the PR @AndriiDiachuk!

I think this is a good start, and there's a little more needed for the usecase. I think we will need 2 new endpoints:

GetTransactionErrorMessage -> Takes a transactionID and blockID, and returns the error message string for that tx.
GetTransactionErrorMessagesByBlockID -> similar to what you have. takes a blockID, and returns a set of objects, one for each tx with an error.

I think the GetTransactionErrorMessagesByBlockID response should contain a repeated message that includes the transaction's ID, index and error string.

[peterargue]

let's create a separate request object for this GetTransactionErrorMessagesByBlockIDRequest

[peterargue]

we'll need a way to differentiate between the messages for different errors.

[AndriiDiachuk]

Thanks for the PR @AndriiDiachuk!

I think this is a good start, and there's a little more needed for the usecase. I think we will need 2 new endpoints:

GetTransactionErrorMessage -> Takes a transactionID and blockID, and returns the error message string for that tx. GetTransactionErrorMessagesByBlockID -> similar to what you have. takes a blockID, and returns a set of objects, one for each tx with an error.

I think the GetTransactionErrorMessagesByBlockID response should contain a repeated message that includes the transaction's ID, index and error string.

Thanks for response @peterargue. Originally, I wanted to implement a few methods like you have suggested but after taking a closer look on current implementation I have noticed that we sync execution data using execution data sync module which syncs error messages only block by block basis. So I thought that we should mirror this logic and sync error messages for all failed transactions in the block. Regarding the response, I've omitted txid and index since we can derive that data by looking at the transactions in block and filtering failed transactions.

For example, if we consider block txns: [A(success), B(failure), C(success), D(failure), E(failure)].
We will get the following response: [err_msg_B, err_msg_D, err_msg_E],
and we can derive txid and index by applying filter func F(res:LightTransactionResult) bool { return res.Failed }.
Main goal is to reduce the size of the response.

If you are worried about consistency of mapping error messages to the actual light execution results we can add a checksum of involved tx ids or a mechanism similar to signer indices(a bitset of tx indexes that are returned). If you think we can neglect with extra space I can always include full tx ids and indexes.

[peterargue]

Thanks for response @peterargue. Originally, I wanted to implement a few methods like you have suggested but after taking a closer look on current implementation I have noticed that we sync execution data using execution data sync module which syncs error messages only block by block basis. So I thought that we should mirror this logic and sync error messages for all failed transactions in the block. Regarding the response, I've omitted txid and index since we can derive that data by looking at the transactions in block and filtering failed transactions.

For example, if we consider block txns: [A(success), B(failure), C(success), D(failure), E(failure)]. We will get the following response: [err_msg_B, err_msg_D, err_msg_E], and we can derive txid and index by applying filter func F(res:LightTransactionResult) bool { return res.Failed }. Main goal is to reduce the size of the response.

If you are worried about consistency of mapping error messages to the actual light execution results we can add a checksum of involved tx ids or a mechanism similar to signer indices(a bitset of tx indexes that are returned). If you think we can neglect with extra space I can always include full tx ids and indexes.

So my thinking on this is:

• Not every Access node/observer will need all tx error messages for every block. The public nodes, nodes that are used with blockchain indexers and other special usecases may, but the majority will only care about a small subset.
• While it may be simplest in an initial version to have every node sync all error messages for every block, this may not scale well. Even if this is how the initial implementation works on ANs, let's still add the endpoint to get the error from a specific tx. I'd rather get the work done now, than have to rescope it later since doing both is minimally more work than just one.
• I see your point about the response size. I worry that if there's a bug or execution fork on an EN, it could returns results for a different set of tx, resulting in inaccurate information cached and returned from the AN. While it does add a little overhead (~40 bytes per tx), the overhead is generally much less than the size of the error messages themselves and allows the AN to validate it got errors for all of the expected tx.

[AndriiDiachuk]

Thanks for response @peterargue. Originally, I wanted to implement a few methods like you have suggested but after taking a closer look on current implementation I have noticed that we sync execution data using execution data sync module which syncs error messages only block by block basis. So I thought that we should mirror this logic and sync error messages for all failed transactions in the block. Regarding the response, I've omitted txid and index since we can derive that data by looking at the transactions in block and filtering failed transactions.
For example, if we consider block txns: [A(success), B(failure), C(success), D(failure), E(failure)]. We will get the following response: [err_msg_B, err_msg_D, err_msg_E], and we can derive txid and index by applying filter func F(res:LightTransactionResult) bool { return res.Failed }. Main goal is to reduce the size of the response.
If you are worried about consistency of mapping error messages to the actual light execution results we can add a checksum of involved tx ids or a mechanism similar to signer indices(a bitset of tx indexes that are returned). If you think we can neglect with extra space I can always include full tx ids and indexes.

So my thinking on this is:

• Not every Access node/observer will need all tx error messages for every block. The public nodes, nodes that are used with blockchain indexers and other special usecases may, but the majority will only care about a small subset.
• While it may be simplest in an initial version to have every node sync all error messages for every block, this may not scale well. Even if this is how the initial implementation works on ANs, let's still add the endpoint to get the error from a specific tx. I'd rather get the work done now, than have to rescope it later since doing both is minimally more work than just one.
• I see your point about the response size. I worry that if there's a bug or execution fork on an EN, it could returns results for a different set of tx, resulting in inaccurate information cached and returned from the AN. While it does add a little overhead (~40 bytes per tx), the overhead is generally much less than the size of the error messages themselves and allows the AN to validate it got errors for all of the expected tx.

@peterargue Made some changes with your suggestions. Waiting for you feedback. Thanks in advance.