validate added

[n13]

🗃 Github Issue Or Explanation for this PR. (What is it supposed to do and Why is needed)

replace naked try/catch with transfer action parser

[chuck-h]

I don't see that this is an improvement.
The only thing we know about the blockchain tx we are handling here is that the action name is transfer. Anyone can write a contract with a transfer action, and we should handle whatever gets thrown at us.
This validator code only tests for a specific type of mismatch: absent fields. However the json we're parsing could have different data types, return structures where we expect a scalar, etc. Lots of ways to fail, and try/catch deals with them all. @n13

[n13]

I disagree - we don't want to "deal with them all" - we only want to deal with the one transfer action that our code can actually handle, which is the one that has all the fields we need.

The try / catch also removes all that don't have the required fields since the fields are non-nullable, and would throw null pointer exceptions in the constructor, it's just implicit.

So to compare try/catch (A) with validate (B):
A:

• dismisses all transfer actions that don't have the required parameters (non-nullable fields set to null causes crash)
• also dismisses any actions that cause unknown errors to happen

B:

• dismisses all transfer actions that don't have the required parameters
• surfaces any other errors we don't know about as crashes

[chuck-h]

We are talking about what shows up on the history list, which is a convenience for the user.

Fundamentally we want the history to show ordinary Telos token transfers and nothing else. (We could also, say, show issue events, like https://wallet.telos.net/?network=telos does, but we choose not to.) I think "transfer" actions from any other type of contract (e.g. NFTs) should be silently ignored. Such blockchain actions are not "errors to be surfaced", they are just a record of the user interacting with other contracts that are beyond our interest.

[n13]

Yeah ok but we never have a naked try/catch with no error printed.

That's kind of the point, not the functionality

Catch is for error conditions

Not for expected conditions. like transfer actions that are other actions than the transfer actions we're interested in.

[chuck-h]

How about we try/catch around the validation, log a catch as an "error" event, and continue (i.e. return null)? That way if, in the future, some new type of unexpected action shows up on the blockchain, we "surface any other errors we don't know about" as logged errors but not as crashes that block the app from displaying the transaction list (disrupt the UX).