Page MenuHomePhabricator

[Cashtab] Implement standardized eCash prefixes in OP_RETURN transactions
Needs ReviewPublic

Authored by emack on Sat, Nov 20, 01:16.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary
  • As per T1967, this diff upgrades the OP_RETURN message logic to propose a Cashtab prefix for messages sent from Cashtab, in addition to the existing prefix which differentiates message transactions from eToken transactions.
  • The parsing logic is also updated to delineate between a Cashtab generated message vs an externally generated message (e.g. electrum) in transaction history.

When sending OP_RETURN messages from within Cashtab, the following script is proposed (see useBCH.js)

OP_RETURN opcode
[Protocol ID] + [Transaction Type]
[Message] - the message content

The following prefix hex codes are proposed in this diff and will be added to an eCash specific prefix standard separately:

opReturn: {
    opReturnPrefixHex: '6a',
    opReturnPushDataHex: '04',
    opReturnAppPrefixLengthHex: '04',
    appPrefixesHex: {
        eToken: '534c5000',
        cashtab: '00746162',
    },
},

When parsing OP_RETURN messages, the parseTxData() logic in useBCH.js is now updated as follows:

  • checks whether an output contains addresses, if not, assume its either a message or eToken tx
  • if the output's hex property contains the eToken prefix, then apply eToken logic
  • if the output's hex property contains the cashtab message prefix, then apply cashtab message rendering logic
  • else, assume it is an external message e.g. from electrum and apply external message rendering logic

image.png (463×495 px, 35 KB)

Test Plan
  • npm start
  • send message from within Cashtab and ensure it is reflected with props.theme.primary styling in tx history across both receiving and sending wallets
  • send message from within Electrum and ensure it is reflected with props.theme.secondary styling in tx history across both receiving and sending wallets
  • test cross browser compatibility in firefox
  • npm run extension
  • verify display in extension mode

Diff Detail

Repository
rABC Bitcoin ABC
Branch
opreturnPrefix
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17428
Build 34684: Build Diffcashtab-tests
Build 34683: arc lint + arc unit

Event Timeline

emack requested review of this revision.Sat, Nov 20, 01:16

Looking good. A couple of changes to keep with the existing standard.

https://github.com/bitcoincashorg/bitcoincash.org/blob/master/spec/op_return-prefix-guideline.md

image.png (290×1 px, 43 KB)

So, 6d02 should instead be 6a04

Compare the cashaccount identifier here: https://github.com/bitcoincashorg/bitcoincash.org/blob/master/etc/protocols.csv

With this OP_RETURN of a cashaccount registration transaction: https://rest.kingbch.com/v4/electrumx/tx/data/d0913d4398b0456cbced94fb1fcc231409cbdbb772eee0a262a349dacacc70d0

Here's another example of a spec conforming tx (Satoshi Dice transaction): https://api.fullstack.cash/v5/electrumx/tx/data/21afd1ed54fd815bd61e0ff7e497457e0f9ad0b3959426000f2a174dc1bc4833

web/cashtab/src/hooks/useBCH.js
962 ↗(On Diff #30928)

Should be 6a04 per spec

963 ↗(On Diff #30928)
  1. Add the Cashtab prefix as an app-wide paramater in the currency object of Ticker.js instead of hard coding it here
  1. Let's go with 0x00746162 as the Cashtab prefix, (zero t a b). So, if I'm reading what's going on here correctly, looks like 313030 should be replaced with 00746162 (keep leading zeros to conform with 4 byte standard)
This revision now requires changes to proceed.Wed, Nov 24, 18:22
emack marked 2 inline comments as done.
  • Prefixes moved to the app-wide currency object parameter in Ticker.js and referenced throughout useBCH.js.
  • Cashtab and OP_RETURN message prefixes updated to align with standard.
  • Added unit tests for the removeOpReturnPrefixes() function.
  1. There are enough OP_RETURN prefixes that we should put them all in their own object inside the currency object, say something like
currency: {
...
...
opReturn : {
  opReturnPrefixHex:
  opReturnPushDataHex:
  cashtabPrefixHex:
  ...
  }
}
  1. See inline comments, some ambiguity between which prefixes are hex and which are asm. It is probably best to only use the hex prefixes. Confusing to have both and have the app scan for different ones in different situations. Mb beyond the scope of this diff to only look at hex and never asm, since that might require refactoring other places -- but that should be the goal unless there is a compelling technical reason to use both.
web/cashtab/src/components/Common/Ticker.js
32 ↗(On Diff #31017)

this is asm, not hex, change name to eTokenPrefixAsm

alternatively, check for the hex prefix, 534c50

See example tx: https://rest.kingbch.com/v4/electrumx/tx/data/7443f7c831cdf2b2b04d5f0465ed0bcf348582675b0e4f17906438c232c22f3d

Vs spec: https://github.com/bitcoincashorg/bitcoincash.org/blob/master/etc/protocols.csv

...spec is super confusing bc all the numbers are backwards. i think we'll change that when we document this for ecash, really want something a dev can copy paste.

33 ↗(On Diff #31017)

From spec at https://github.com/bitcoincashorg/bitcoincash.org/blob/master/spec/op_return-prefix-guideline.md

However, values pushed on the Script stack are expected to be encoded in little endian. Thus, for a protocol identifier 0x0ABCDE01, the first bytes of the script are expected to be { 0x6a, 0x04, 0x01, 0xDE, 0xBC, 0x0A } where 0x6a encodes for OP_RETURN.

I think it's most readable to split this up a bit. Documentation is so poor for OP_RETURN that this would be helpful for new devs seeing the repo.

So, use these parameters:

opReturnPrefixHex: 6a
opReturnPushDataHex: 04

35 ↗(On Diff #31017)

This is the hex one, call it cashtabPrefixHex

web/cashtab/src/hooks/__mocks__/mockParsedTxs.js
68 ↗(On Diff #31017)

This one is labeled both isCashtabMessage: false and isCashtabMessage: true

96 ↗(On Diff #31017)

Repeated object key isCashtabMessage

This revision now requires changes to proceed.Thu, Nov 25, 17:42
emack marked 5 inline comments as done.

Adjustments to prefix references and mock attributes

bytesofman added inline comments.
web/cashtab/src/components/Common/Ticker.js
32 ↗(On Diff #31052)

Evaluate if its technically feasible to only use the hex values here and ignore asm. If so, lose the asm stuff and go hex only.

36 ↗(On Diff #31052)

Not sure we need this one

This revision now requires changes to proceed.Thu, Nov 25, 22:35
emack marked 2 inline comments as done.
  • refactored parseTxData() function in useBCH.js to only reference hex attributes instead of asm.
  • hex parsing logic are encapsulated in Ticker.js, enhancing the readibility of the prasing logic for eToken/Cashtab message/External message
  • unit tests added for the new parsing functions in Ticker.js
bytesofman added inline comments.
web/cashtab/src/components/Common/Ticker.js
33 ↗(On Diff #31065)

Make a new appPrefixesHex: {} object as a child of the opReturn object that, for now, includes eToken and cashtab: , instead of current arrangement with eTokenPrefixHex: and cashtabPrefixHex as separate entries in the opReturn: object

This will make Cashtab more extensible if we see future apps adopting other unique prefixes and wish to parse for those as well.

85 ↗(On Diff #31065)

Desired output here is

6a0400746162

6a = OP_RETURN
04 = next thing on the stack is 4 bytes long
00746162 = cashtab's 4-byte prefix

88 ↗(On Diff #31065)

A couple of issues here.

  1. This value should be 04 and not 02 (the number of bytes in an OP_RETURN app prefix, in hexadecimal)
  1. Avoid any hardcoding a hex string without any other explanation of what it is here.

I don't think it makes sense to store prefixes for all the possible hex variations of "next thing up is this many bytes". For readability though, it would be better to use a new function, something like getByteCountHex(), where input is a base 10 number and output is required hex string in op code format.

For now...we only need this value to indicate next thing up on the stack is the prefix. So adding opReturnAppPrefixLengthHex: '04' to currency object is good for this diff.

111 ↗(On Diff #31065)

Note that what is left of the message after OP_RETURN + prefix byte count + correct prefix will also include 2 bytes that describe how many more bytes are on the stack

For example, you could have

a048657265206973206120707265747479206c6f6e67206d6573736167652073656e74207769746820436173687461622e20486d2c20746869732076616c69646174696f6e2066756e6374696f6e2069732077617920746f6f20736c6f772e204d6179626520746f6f206d756368206973206265696e672072652d72656e64657265642077697468206576657279206368617261637465723f2049732074686973

Breaks down into

a0 -> next thing on the stack is 160 bytes
48657265206973206120707265747479206c6f6e67206d6573736167652073656e74207769746820436173687461622e20486d2c20746869732076616c69646174696f6e2066756e6374696f6e2069732077617920746f6f20736c6f772e204d6179626520746f6f206d756368206973206265696e672072652d72656e64657265642077697468206576657279206368617261637465723f2049732074686973 -> Here is a pretty long message sent with Cashtab. Hm, this validation function is way too slow. Maybe too much is being re-rendered with every character? Is this

can inspect that tx (sent with prod Cashtab, before this diff landed) here: https://rest.kingbch.com/v4/electrumx/tx/data/de6640fdbd8e1b51e85c20754e9da3227b90c47f11e6714d8948177713a87dd6

Important to note that when this diff lands, it will not correctly parse old Cashtab msgs, because they are not constructed correctly per spec. That's okay. We only want to parse for spec-correct msgs.

web/cashtab/src/components/Common/__tests__/Ticker.test.js
122 ↗(On Diff #31065)

The desired output here is

6a0400746162

132 ↗(On Diff #31065)

These need to be updated to reflect corrected prefix above

web/cashtab/src/hooks/useBCH.js
1007 ↗(On Diff #31065)

correct this per prefix corrections above

This revision now requires changes to proceed.Fri, Nov 26, 22:35
emack marked 7 inline comments as done.
emack edited the summary of this revision. (Show Details)
  • Updated Ticker.currency.opReturn with a new appPrefixesHex object
  • Adjusted the cashtab prefix used in getCashtabEncodingSubstring() and constructing the OP_RETURN script in sendBch()
  • Adjusted unit tests to align with updated prefix spec