Details

Reviewers

bytesofman

Group Reviewers

Restricted Project

Commits

rABCb776dcd016c4: [Cashtab] [OP_RETURN msg upgrade] - Pt2 - Use bytecount for unencrypted airdrop…

Summary

T3100

Depends on D13729

Updates the max message limit to reflect the reduced OP_RETURN script space for airdrop transactions since the eToken ID is embedded into the OP_RETURN output.

Test Plan

calculate an airdrop and click Copy to send screen
expand the message section and ensure the placeholder text reflects 38 bytes less than the unencrypted message limit
enter more than 177 bytes and ensure validation error is triggered
enter a message of 177 or less bytes, click Send and ensure successful broadcast
navigate away and then back to the Send screen without any airdrop context, and ensure the placeholder text in the message section is showing the full 215 byte limit
enter a message of 215 bytes or less and ensure no regression
send a normal XEC tx with no message and ensure no regression
send a one to many XEC tx with no message and ensure no regression
send a one to many XEC tx with a 215 byte message and ensure no regression
grep -r 'unencryptedAirdropMsgByteLimit' src/ and ensure no instances of unencryptedAirdropMsgByteLimit

Diff Detail

Repository

rABC Bitcoin ABC

Branch

opreturnUpgrade

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 23520
Build 46659: Build Diff	cashtab-tests
Build 46658: arc lint + arc unit

Event Timeline

emack created this revision.Apr 27 2023, 13:25

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 27 2023, 13:25

emack requested review of this revision.Apr 27 2023, 13:25

Harbormaster completed remote builds in B23520: Diff 40026.Apr 27 2023, 13:58

bytesofman requested changes to this revision.Apr 27 2023, 15:18

bytesofman added inline comments.

cashtab/src/components/Send/Send.js
613	Break this out <tokenId> is 32 bytes Pushdata for CashtabMsg prefix is 2 bytes CashtabMsg prefix is 4 bytes Ideally define these as variables and avoid magic number "38"
1034	esp since we are using 38 again here.

This revision now requires changes to proceed.Apr 27 2023, 15:18

rebase

Harbormaster completed remote builds in B23547: Diff 40066.Apr 28 2023, 11:39

Breaking airdrop max byte size down to individual params for calculation

Harbormaster completed remote builds in B23549: Diff 40068.Apr 28 2023, 12:52

bytesofman requested changes to this revision.Apr 28 2023, 13:26

bytesofman added inline comments.

cashtab/src/components/Common/Ticker.js
108–110 ↗	(On Diff #40068)	Let's keep this out of Ticker.js
cashtab/src/components/Send/Send.js
614 ↗	(On Diff #40068)	define variables and add to 38 here. variables const pushDataByteCount = 1 const prefixByteCount = 4 const tokenIdByteCount = 32 const airdropTxAddedBytes = pushDataByteCount + tokenIdByteCount + pushDataByteCount + prefixByteCount //38

This revision now requires changes to proceed.Apr 28 2023, 13:26

additional OP_RETURN output byte usage calculation moved to useEffect() loop so it's available to both handleMsgChange and the frontend render

Harbormaster completed remote builds in B23568: Diff 40098.Apr 29 2023, 03:30

emack added a child revision: D13827: [Cashtab] [OP_RETURN msg upgrade] - Pt3 - Use bytecount for encrypted messages.Apr 29 2023, 05:49

bytesofman requested changes to this revision.Apr 29 2023, 17:04

bytesofman added inline comments.

cashtab/src/components/Send/Send.js

180 ↗

(On Diff #40098)

We don't ever want this to be zero though right? Should always be 38.

No need for a state variable. Instead, define a constant here.

// Airdrop transactions embed the additional tokenId (32 bytes), along with prefix (4 bytes) and two pushdata (2 bytes)// hence setting airdrop tx message limit to 38 bytes less than currency.opReturn.unencryptedMsgByteLimit
const pushDataByteCount = 1;
const prefixByteCount = 4;
const tokenIdByteCount = 32;
const localAirdropTxAddedBytes = pushDataByteCount + tokenIdByteCount + pushDataByteCount + prefixByteCount; //38

271–277 ↗

(On Diff #40098)

remove

This revision now requires changes to proceed.Apr 29 2023, 17:04