4byte: Enrich transaction requests and activities with ABIs

[mhluongo]

This PR covers the background script components of #492. Each transaction and transaction signature request is enriched by looking up its function signature in the 4byte.directory database. These signatures can be used to (eventually) replace our bland "Contract interaction" interface with something contract-specific.

Here's the data from the last few contract interactions in one of my wallets.

While I was at it, I found and fixed a major enrichment bug that replaced contract names for generic contract interactions with the name of the sender.

To Test

• [ ] Interact with a contract from an address that resolves to an ENS name. Make sure the activity shows the contract info, not the senders.
• [ ] Load a read-only wallet with an interesting history — maybe LP'ing, registering an ENS name, or voting on-chain. Watch the network tab, and you should see 4byte.directory API calls
• [ ] Confirm the calls aren't for the same hex selector — none should appear twice thanks to caching.
• [ ] Check the Chrome background script console, and confirm that selectors are cached under "IndexedDB > tally/enrichment"
• [ ] Finally, confirm the Redux store has a functionSignature for many (though not all) activities with type === "contract-interactions"

[mhluongo]

Now the meaty one. IMO @0xDaedalus's point about collisions means this whole code path should be disconnected from enrichment until we've implemented the additional bits you've described @mhluongo. Initially I was going to say it depends on how we want to implement it in the UI (is it a potentially inaccurate aid or a security feature?), but until we know that I feel like it's subject to knowledge rot that will eventually make it seem like a good security feature when it is currently arguably more of a vector.

Did you see my follow-up @Shadowfiend? It wouldn't just have to be a function selector collision... it would also have to be the first time the selector has been seen by the service, ever.

If the concern is newly deployed contracts, we can set a date after which we don't accept 4byte submissions by themselves. Say, today. That way, we can get useful human-readable metadata in the wallet while the bigger metadata push above is implemented... without opening up a vector for manipulation.

[Shadowfiend]

The issue is creating a malicious function whose sighash collides with a matching 4byte signature, not vice versa, no?

[0xDaedalus]

Yes - what we don't want to happen (and what I believe can happen in the current implementation) is:

• User interacts with malicious contract
• Malicious function signature matches that of a trusted function
• Our application identifies to the user that the function trusted (because 1st matching 4byte signature is trusted)
• Malicious function does malicious things

[mhluongo]

All of that is a UI concern though. This PR doesn't include an interface that suggests this information should be trusted as correct — it gives us the information necessary to approach feature parity with MetaMask, which is already vulnerable to that same attack.

You two are suggesting we shouldn't store spoofable information, rather than that we should very carefully design an interface to explain the risk. We aren't holding to that threat model throughout the rest of the extension though, or we wouldn't be relying on eg trusted RPCs.

Happy to rename variables here to make the intent more clear, or include more documentation on collisions, or both...? Also happy to add a feature flag here, and happy to add a cutoff date that solves 40% of the collision issue. But this isn't a UI PR, and solving all of these concerns is going to be way more work than a single PR's worth of review.

Thoughts?

[Shadowfiend]

I had the mind flash of a variable rename the other day but forgot it before suggesting. I'd be fine with a rename that made it clear this wasn't a known good value.

[0xDaedalus]

Very pro variable rename 👍

[0xDaedalus]

I'd like to take the foot off the gas a bit on the transaction enrichment path until we get some pending stability and plumbing enhancements in.

If we want to do a variable rename as suggested above, and add a feature flag that turns this off until we are ready for it - very happy to merge with those additions.

[mhluongo]

Moving this back to draft and giving it a rethink.

[Shadowfiend]

Closing this for now; we'll be doing a rework to pull in transaction simulations soon, and this may factor into that work as a fallback or similar.