Skip to content

Tx input parsing#16

Draft
bejitono wants to merge 12 commits intoSafari-Wallet:mainfrom
bejitono:tx-input-parsing
Draft

Tx input parsing#16
bejitono wants to merge 12 commits intoSafari-Wallet:mainfrom
bejitono:tx-input-parsing

Conversation

@bejitono
Copy link
Copy Markdown
Collaborator

@bejitono bejitono commented Dec 22, 2021

Tx List Screenshot:
Screenshot 2021-12-26 at 19 49 17

Tx Detail Screenshot:
Screenshot 2021-12-26 at 19 49 29

This PR implements transaction input parsing. It parses the tx input in order to retrieve the method that was invoked along with the call arguments.

The approach is as follows:

  • Fetch tx list from Unmarshal (currently looking at Zerion API)
  • Fetch tx list from Etherscan which includes tx input data (and merge with Unmarshal txs)
  • For each tx, fetch contracts from Etherscan that includes the ABI
  • For each tx , decode input data to obtain the method name and its decoded input parameters (based on MewWalletKit)
  • Get description from a curated list (MethodInfo.plist) that defines how to format input parameters into a useful description (e.g. “Registered ENS XYZ”) based on contract address and method hash or show the method name (TBD)

<key>Description</key>
<dict>
<key>StringFormat</key>
<string>Registered %@</string>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you guys think of this approach? Maintaining a list of string formats per contract/methodId so we can display the tx input in a descriptive way, instead of dumping the method name and its arguments? But it's something we'd have to maintain over time...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maintaining a list is a good idea. While it involves more upfront work, and the ongoing maintenance cost, it's the best approach we have so far to provide really clear, user-friendly descriptions of transactions.

One question: why a plist? Would it not be better to put this in Swift directly, perhaps a map from address+method to a callback? This way, we can not only return text, but also other pieces of UI (logos, links, perhaps even in-wallet actions?)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to our discussions, this approach is still a bit limited as it assumes that the parameter value’s string representation can be readily displayed. However, in some cases we will need to do some further formatting, e.g. when the input parameter is a timestamp, we would like to format it in a readable way. One way we could handle this is to add an additional type argument which tells the client how to format the value. wdyt?

Copy link
Copy Markdown
Contributor

@DimitarNestorov DimitarNestorov Dec 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I agree. What about using String(format:_:), or another string formatting API, or a library? https://riptutorial.com/swift/example/6342/formatting-strings

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! So in the current PR String(format:arguments:) is used (see TransactionInputParser). So we should be able to handle different format specifiers. But if we wanted to do date formatting for example then string formats wouldn't be sufficient anymore.

There are also a couple of other things to consider:

  • The format specifier used in the string format has to match the type of the decoded input param.
  • The list of arguments provided in the plist/json to be in the right order. Also each argument in the plist/json needs to match the param name of the decoded input.

It's easy for errors to creep in here. When the list contains errors in the string format or the supplied argument names then it will be difficult to catch. We will need to think about how we can add further validation perhaps, and/or how to properly test updates in the string format list.

}
}

private func convertRawValueToString(inputData: InputData) -> String? {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just simple conversion to string values right now so we can display it on the tx history detail page for demo purposes.

<key>Description</key>
<dict>
<key>StringFormat</key>
<string>Registered %@</string>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to our discussions, this approach is still a bit limited as it assumes that the parameter value’s string representation can be readily displayed. However, in some cases we will need to do some further formatting, e.g. when the input parameter is a timestamp, we would like to format it in a readable way. One way we could handle this is to add an additional type argument which tells the client how to format the value. wdyt?

@ronaldmannak
Copy link
Copy Markdown
Contributor

This is an interesting problem :) It might make sense to add a field with a format description or maybe even a regex for odd cases.

However, timestamps are especially tricky and I don't know if a regex is the best approach here. I think it will be a two step process:

  1. Convert date representing string to a Swift Date object
  2. Localize the Date object to a human readable string (e.g. mm/dd/yyyy in the US and dd/mm/yyyy for the rest of the world, or something like that).

Step 2 is relatively straightforward. DateFormatter has been around for a long time, and Swift 5.5 has a new date formatter system I haven't tried yet but looks promising.

Step 1 is a little bit more complicated because there are just so many different ways to store dates in a string (and they claim to be "standard").

I wonder if we could add a field date parameters in the plist that stores the date format that's easy to parse by DateFormatter or the new Swift formatting methods. Maybe the field should say something like "yyyy-MM-dd'T'HH:mm:ssZZZZZ" which DateFormatter can easily parse directly. Alternatively, we could store a string internally maps to an Enum: String with values like "ISO8601" or "RFC3339". However, if some way too smart web3 developer developed his own (and presumably "much better") date format, you probably want to use the first approach which is more flexible. (Given the weird code I have seen in blockchain development, flexibility is probably the way to go here)

@bejitono bejitono marked this pull request as draft January 16, 2022 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants