Page MenuHomePhabricator

[Cashtab] Upgrade transaction history to parse and display OP_RETURN messages
AbandonedPublic

Authored by emack on Oct 30 2021, 13:43.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary

End to end updates to tx history logic to enable the parsing of OP_RETURN outputs and render them as part of the Transaction History tab.

  • Updated useBCH.js's parseTxData function to extract the asm value from the getTxData JSON response, which is then parsed to determine whether it is an eToken tx with OP_RETURN outputs or a genuine OP_RETURN message encoded output
  • The message content is then decoded into text and returned as part of parseTxData() back all the way up to Tx.js, which conditionally renders the message if it exists
  • Updated mockParsedTxs.js and mockTxDataWithPassthrough.js to incorporate the new opReturnMessage parameter that is now being returned from parseTxData() in useBCH.js, as well as new message-centric mocks
  • Need further discussion with @bytesofman on how best to guard against OP_RETURN outputs generated by custom nodejs scripts as they wouldn't be going through the front and backend validtion logic in Cashtab which may cause cashtab to crash if getTxData() returns an unexpected JSON response.

Deployment Order

  1. Propagate T1920's null utxo fix to extension plugin
  2. Land D10363
  3. Then land this Diff
Test Plan
  • arc patch D10363 to utilise the draft Send OP_RETURN message function under review
  • npm start
  • in-app sending of outbound OP_RETURN tx from address A to address B with normal message
  • in-app sending of outbound OP_RETURN tx from address B to address A with normal message
  • Force cashtab to refresh tx history instead of retrieving from local storage by sending a normal outbound tx from address A
  • verify both inbound and outbound OP_RETURN messages are displayed in the Transaction History tab for both wallets and not for other non-message txs
  • in-app sending of outbound OP_RETURN tx from address A to address B with blank message. Verify this is not displayed in Cashtab as D10363 ignores empty string messages
  • manual sending of outbound OP_RETURN tx from address A to address B with a message containing over 150 characters (D10363 limits to 150 chars and below) and verify this is displayed appropriately by being cut off at appropriate height and width limits. This is to guard against mischievious users purposely spamming insanely long messages via nodejs to other users' tx history
  • manual sending of outbound OP_RETURN tx from address A to address B with a message containing an extra long word and verify this is displayed appropriately w.r.t. word break and height clipping CSS
  • send normal outbound token tx and ensure the new OP_RETURN parsing logic in useBCH.js does not break existing eToken parsing logic
  • verify cross-browser compatibility in Firefox
  • npm run extension
  • verify OP_RETURN messages are displayed in extension mode

Diff Detail

Repository
rABC Bitcoin ABC
Branch
parseOpReturnTx
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17186
Build 34203: Build Diffcashtab-tests
Build 34202: arc lint + arc unit

Event Timeline

emack requested review of this revision.Oct 30 2021, 13:43
bytesofman requested changes to this revision.Nov 2 2021, 18:24

Good start. I think we need to be more generic in the parsing approach: parse anything that isn't prefixed as an eToken tx (see inline comments).

web/cashtab/src/hooks/__mocks__/mockParsedTxs.js
91 ↗(On Diff #30717)

Is this an IDE change?

web/cashtab/src/hooks/__mocks__/mockTxDataWithPassthrough.js
768 ↗(On Diff #30717)

where did all the formatting changes in this file come from?

web/cashtab/src/hooks/useBCH.js
142 ↗(On Diff #30717)

I think we need to take the opposite approach here -- instead of testing for the 621, which is in some msgs but not others, we should test for SLP -- which is in all valid token transactions.

For example these eToken txs all have the same prefix

https://explorer.be.cash/tx/5a12af320209b76ff7f827d16b914e7acd8087fb9912c0d526abb1eab6780447
https://explorer.be.cash/tx/e26db37d5c64b265514cd5cbb9d5194a7f2967b5974d167236d46be4954e435c
https://explorer.be.cash/tx/98183238638ecb4ddc365056e22de0e8a05448c1e6084bae247fae5a74ad4f48

Either OP_RETURN 5262419 in asm or 6a04534c500001010 in hex.

So, approach should be to check if this tx is trying to be an eToken tx first. If it is, assume tokenTx is true -- we don't need to parse the msg of an eToken tx or one impersonating it. It will just show up as an invalid eToken tx.

145 ↗(On Diff #30717)

Not all msgs will be in `asm.split(' ')[2], e.g. here is that field for this txid

txid: 5b81b332c8fa5a2b2e77bb928bd18072af4485f02a7325d346f1f28cf3d4a6bb

asm: 'OP_RETURN 4a6f686e204d6341666565206973207468652073616765206f662074686520323173742063656e747572792e'

But we should still be able to parse this msg.

Might be better to go for the hex and parse the whole thing for any msg. Then we can have a series of filters to decode and flag different prefixes.

This revision now requires changes to proceed.Nov 2 2021, 18:24
emack marked 4 inline comments as done.Nov 3 2021, 07:04
emack added inline comments.
web/cashtab/src/hooks/__mocks__/mockParsedTxs.js
91 ↗(On Diff #30717)

this was suggested by prettier

web/cashtab/src/hooks/__mocks__/mockTxDataWithPassthrough.js
768 ↗(On Diff #30717)

I added in two new transactions (from line 767 till end of file) to represent txs with OP Return message outputs, which were pasted in directly from a JSON formatter, which had the quotation marks. Prettier asked for it to be removed in line with formatting rules.
This isn't that old case of prettier asking for untouched files to be reformatted.

emack marked 2 inline comments as done.
  • revised OP_RETURN encoded message parsing logic
  • In a future diff for NFTs, this will be further adjusted to differentiate between eToken and NFT1 tx
  • new removeOpReturnPrefixes() function added that strips out the nominated prefixes. If they don't exist then the original string is returned untouched.