As per T2288, updated Send component and useBCH hook to detect an airdrop payment and adds an airdrop standard prefix and message to the OP_RETURN output.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABCec89448bdfd2: [Cashtab] Adding airdrop hex prefix and OP_RETURN msg to airdrop payments
- npm start
- send non-airdrop one to many XEC Send with no message and verify no OP_RETURN output.
- send non-airdrop one to many XEC Send with a message and verify the following OP_RETURN output:
OP_RETURN
_tab
the message
- generate an airdrop, click on the Copy to Send Screen button, then send without a message and verify the following OP_RETURN output:
OP_RETURN
drop
_tab
- generate an airdrop, click on the Copy to Send Screen button, then send with a message and verify the following OP_RETURN output:
OP_RETURN
drop
_tab
the message
- ensure no regression with the encrypted (etab in place of _tab) and unencrypted one to one transactions.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Nice.
I like the idea of having a standard prefix for 'airdrop' and then combining that with another indicator to show which app was used to send the airdrop.
However, we want to stick with the 4-char standard for an OP_RETURN app prefix. So, we should pick one generic prefix that comes first and denotes that this is an airdrop transaction. Sticking with 64726f70 for drop is great.
We don't need 41697264726f70205472616e73616374696f6e (Airdrop Transaction) as this can be inferred from the drop prefix.
- So, overall change:
a) Prefix should be 64726f70 and this should come first b) Add the cashtab prefix as a modifier after this initial prefix. Other apps in the future may also create and send airdrop transactions. They should use the drop prefix, but can be differentiated from Cashtab in that they won't use the Cashtab supplementary identifier.
Current:
6a (OP_RETURN) 04 00746162 ( tab) 13 41697264726f70205472616e73616374696f6e (Airdrop Transaction) 04 64726f70 (drop)
Requested:
6a (OP_RETURN) 04 64726f70 (drop) 04 00746162 ( tab)
I'm not against using the extra space we have available in OP_RETURN here to add more info about the tx that is not strictly available from a block explorer. However, we should not include "Airdrop Transaction" as this may be inferred from the prefix -- this is the point of the prefix system, to efficiently use blockspace.
I don't think we have a good case for this yet on Cashtab. I can see in the future wanting to include more specific information about certain types of transactions, for example if one token sent different types of dividend payments. In the meantime though we should keep the space available.
- This diff should also update web/standards/op_return-prefix-guideline.md
Comment Actions
- Updated diff to position the airdrop prefix as per feedback.
- Parsing logic in parseTxData retrieves the airdrop prefix and then removes it from array so that the complex parsing logic for cashtab/external/encryption parsing can remain unchanged.
- Unit tests updated accordingly.
- Added airdrop prefix to standards page
Onchain samples
- Airdrop tx with no message
https://explorer.be.cash/tx/8fb79674fe1c84fc5adb039097ee281876715bc726ef4aa8ac9576959d96c75c
6a
04
64726f70 drop
04
00746162 tab
- Airdrop tx with message
https://explorer.be.cash/tx/8c992b638135e54bd9c1a911e71e8282c014f04dfd142a601a6b48d745a707aa
6a
04
64726f70 drop
04
00746162 tab
0f
61697264726f70206d657373616765 message
- Non-Airdrop tx with message
https://explorer.be.cash/tx/92b3dfd7ae9764d14a02b8db7720293f690673860be3b89d19ff4a38f928fa85
6a
04
00746162 tab
13
6d756c7469322077697468206d657373616765 message
- Non-Airdrop tx without message
https://explorer.be.cash/tx/4777f8f593eef611b5ea6d408b57889711eee53bae9d55288f43bbb2847cd004
76a914f627e51001a51a1a92d8927808701373cf29267f88ac (no OP_RETURN prefix)
- regression tested one to many send with and without an OP_RETURN message
- regression tested one to one send with and without message/encryption
Comment Actions
At some point I'll refactor sendXec() so the logic constructing the script array is more efficient across all the permutations, but since that needs a big regression test I'll create a task to do this separate to this diff.
Comment Actions
- Inline comments
- To confirm -- it looks like this diff adds the parsing logic to parse airdrop txs in the tx history, but does not add the UI elements to display this? That split is ok since we have a task for that, just making sure I'm reading this correctly.
| web/cashtab/src/hooks/useBCH.js | ||
|---|---|---|
| 190 ↗ | (On Diff #33290) | should this be airdropFlag = true ? Is aidropFlag not a boolean here? |
| 192 ↗ | (On Diff #33290) | Explain in comment that this allows special Cashtab-specific case of airdrop tx with a Cashtab msg |
Comment Actions
yes correct, this diff adds the parsing logic to parse airdrop txs in the tx history, without the UI elements. Only goes as far as populating the parsedTx.airdropFlag attribute in parseTxData() so that whoever is working on the UI part can simply check this in Tx.js and render accordingly.
| web/cashtab/src/hooks/useBCH.js | ||
|---|---|---|
| 190 ↗ | (On Diff #33290) | airdropFlag is not a boolean, it is only false by default. The parseOpReturn function in cashMethods returns the airdrop hex prefix here. |
| web/cashtab/src/hooks/useBCH.js | ||
|---|---|---|
| 192 ↗ | (On Diff #33308) | I'm still not getting what this is used for. If this is not a boolean and not used for the same reasons as airdropFlag in sendBCH and the UI components, please rename and add some comments to the code about how this is used. |