⚙ D20597 [Cashtab] Parse XecVibe OP

Owners added a reviewer: Restricted Owners Package.Tue, Sep 8, 21:44

a2c.dev requested review of this revision.Tue, Sep 8, 21:44

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Always code as if the person who ends up maintaining your code is a violent psychopath who knows where you live. - John Woods

────────────────────────────────────────────────────────────────────────

major [Functional Correctness]
→ ]8;;vscode://file//work/modules/ecash-parse/src/parseTx.ts:666modules/ecash-parse/src/parseTx.ts:666]8;;

Reject malformed UTF-8 before creating the XecVibe action.

The XecVibe branch decodes stackArray[1] with Buffer.toString('utf8'),
checks only its byte length, and then creates isValid: true. The payload
6a045845435601ff supplies ff as the memo, which decodes to U+FFFD and
passes the length check. Use fatal UTF-8 decoding and add this payload as
an invalid fixture.

────────────────────────────────────────
Review complete
1 finding ✔

Major 1

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

Harbormaster completed remote builds in B38117: Diff 61264.Tue, Sep 8, 21:51

The coderabbit review comment is good, please follow it. When updating the diff, please be sure to follow the guidelines in CONTRIBUTING.md i.e. do not create a new commit, amend the existing commit.

This revision now requires changes to proceed.Wed, Sep 9, 06:41

Reject malformed UTF-8 XecVibe memos (CodeRabbit).

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Veni, Vidi, Validavi. I came, I saw, I reviewed.

────────────────────────────────────────────────────────────────────────

major [Functional Correctness]
→ ]8;;vscode://file//work/modules/ecash-parse/src/parseTx.ts:665modules/ecash-parse/src/parseTx.ts:665-666]8;;

Validate that the memo is a data push.

A script such as 6a045845435651 has bare OP_1 after the XECV LOKAD.
getStackArray exposes it as 51, so this code accepts it as the UTF-8
memo "Q". The protocol requires a pushdata operation, not an arbitrary
opcode.

Retain script-operation metadata and reject a second element that is not a
data push. Add this script to xecvParseFixtures as invalid.

────────────────────────────────────────────────────────────────────────

major [Functional Correctness]
→ ]8;;vscode://file//work/cashtab/src/opreturn/index.ts:249cashtab/src/opreturn/index.ts:249]8;;

Reject non-push script opcodes.

getStackArray normalizes bare opcodes into hex strings. For example,
045845435651 contains bare OP_1, but this line decodes 51 as memo
"Q" and marks the payload valid. This violates the XecVibe requirement
for exactly one pushdata after the LOKAD.

Parse the script at the operation level and require that the memo element
is a data-push. Add 045845435651 as an invalid regression case.

────────────────────────────────────────
Review complete
2 findings ✔

Major 2

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

Harbormaster completed remote builds in B38132: Diff 61292.Wed, Sep 9, 14:48

latest coderabbit replies are also good, implement

This revision now requires changes to proceed.Wed, Sep 9, 14:50

Reject bare opcodes as XecVibe memo (require data push).

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Minimize coupling between modules. Avoid coupling by writing 'shy' code and applying the Law of Demeter.

────────────────────────────────────────
Review complete
No findings ✔

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Harbormaster completed remote builds in B38133: Diff 61294.Wed, Sep 9, 15:43

Please make the first line of the commit message shorter.

Super-long summary makes it difficult to quickly tell what this Diff is doing

Shorten commit subject per Mengerian.

a2c.dev retitled this revision from [Cashtab] Parse XecVibe (XECV) OP_RETURN prefix Register LOKAD 58454356 for XecVibe payments via Cashtab BIP21. Layout: XECV + memo. Parse in Send preview and ecash-parse history. to [Cashtab] Parse XecVibe OP_RETURN.Wed, Sep 9, 17:09

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) I leave no bug behind. Semper Fixis.

────────────────────────────────────────
Review complete
No findings ✔

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Harbormaster completed remote builds in B38134: Diff 61295.Wed, Sep 9, 17:16

Please fix the linter violations

Fabien requested changes to this revision.Wed, Sep 9, 17:45

This revision now requires changes to proceed.Wed, Sep 9, 17:45

Fix Prettier lint in opreturn vectors (Fabien).

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) You used any like it's a life jacket. Spoiler: it's a pool noodle.

────────────────────────────────────────
Review complete
No findings ✔

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Harbormaster completed remote builds in B38137: Diff 61298.Wed, Sep 9, 18:21

Fabien added inline comments.Wed, Sep 9, 19:38

doc/standards/xecvibe.md
21 ↗	(On Diff #61298)	v0? If you want more versions later you need a version byte somewhere otherwise there is no way to distinguish between versions

Clarify fixed wire layout; drop v0 (no on-chain version byte).

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Nothing is more permanent than a temporary solution.

────────────────────────────────────────
Review complete
No findings ✔

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Harbormaster completed remote builds in B38138: Diff 61299.Wed, Sep 9, 20:00

bytesofman requested changes to this revision.Thu, Sep 10, 08:01

bytesofman added inline comments.

doc/standards/xecvibe.md
29–31 ↗	(On Diff #61299)	This is an AI explanation for a design decision that was actually no decision. It's fine, we can parse it in Cashtab. But it's not acceptable to change LOKAD every time you want to change something about your app. Having AI make that up and establish it as a precedent in this diff is also not ok.

This revision now requires changes to proceed.Thu, Sep 10, 08:01

Simplify xecvibe.md; drop LOKAD-upgrade wording.

Harbormaster completed remote builds in B38141: Diff 61310.Thu, Sep 10, 11:30

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Be a super developer. Go home early.

────────────────────────────────────────
Review complete
No findings ✔

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

bytesofman commandeered this revision.Thu, Sep 10, 13:01

bytesofman edited reviewers, added: a2c.dev; removed: bytesofman.

Parse XecVibe from stackArray instead of rebuilding the script.

pending further review / clean up

Keep the OP_1 stackArray vector; expect memo Q.

Fabien requested changes to this revision.Thu, Sep 10, 13:26

Fabien added inline comments.

cashtab/src/opreturn/index.ts
250 ↗	(On Diff #61310)	it's already unstacked and available in stackArray at this point
doc/standards/xecvibe.md
30–31 ↗	(On Diff #61310)	I don't understand the sentence in this parenthesis
57 ↗	(On Diff #61310)	That section should not be there since it doesn't add any spec
modules/ecash-parse/src/fixtures/xecvFixtures.ts
154 ↗	(On Diff #61310)
307 ↗	(On Diff #61310)	Showing the hex makes no sense If it's expected to be hex data why would you make it utf-8 ? FYI this is "O 5�J�܅mP�0�" Memo is expected to be the decoded utf-8 so either this is encoded hex chars (which is pointless) or a bug
modules/ecash-parse/src/parseTx.ts
160 ↗	(On Diff #61310)	you don't need that when you already have the stackArray

This revision now requires changes to proceed.Thu, Sep 10, 13:26

Address Fabien: stackArray parse, drop BIP21/PUSHDATA1 asides, utf8 memo fixtures.

Harbormaster completed remote builds in B38146: Diff 61316.Thu, Sep 10, 13:31

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) I am the right brain to your SAST's left brain.

────────────────────────────────────────────────────────────────────────

minor [Functional Correctness]
→ ]8;;vscode://file//work/cashtab/src/opreturn/index.ts:260cashtab/src/opreturn/index.ts:260-262]8;;

Preserve a leading BOM in the memo.

The default TextDecoder removes a leading U+FEFF from efbbbf616263.
This changes the valid memo value from \uFEFFabc to abc. Set
ignoreBOM: true in both memo decoders, including
modules/ecash-parse/src/parseTx.ts at Lines 675-677. Add a regression
vector for 045845435606efbbbf616263 with memo \uFEFFabc.


Proposed fix

- memo = new TextDecoder('utf-8', { fatal: true }).decode(
+ memo = new TextDecoder('utf-8', {
+     fatal: true,
+     ignoreBOM: true,
+ }).decode(
      memoBytes,
  );

────────────────────────────────────────
Review complete
1 finding ✔

Minor 1

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

Use stackArray.length !== 2 instead of index typeof checks.

Harbormaster completed remote builds in B38150: Diff 61320.Thu, Sep 10, 14:03

Build Bitcoin ABC Diffs / Diff Testing (ai-review) failed.

Tail of the build log:

/work /work/abc-ci-builds/ai-review
Connecting to CodeRabbit... 0s elapsed
Preparing review... 1s elapsed

  ✗ Review limit reached

  Limit details: You’ve used all 5 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Check out review usage here.
  You can wait 11 minutes for the limit to reset.
  https://app.coderabbit.ai/settings/billing?tab=usage&orgId=79839cd4-7849-4bd5-8d6d-5ecaf453356e
  https://app.coderabbit.ai/dashboard/review-capacity?orgId=79839cd4-7849-4bd5-8d6d-5ecaf453356e

Build ai-review failed with exit code 1

Harbormaster completed remote builds in B38151: Diff 61321.Thu, Sep 10, 14:05

Build Bitcoin ABC Diffs / Diff Testing (ai-review) failed.

Tail of the build log:

/work /work/abc-ci-builds/ai-review
Connecting to CodeRabbit... 0s elapsed
Preparing review... 1s elapsed

  ✗ Review limit reached

  Limit details: You’ve used all 5 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Check out review usage here.
  You can wait 10 minutes for the limit to reset.
  https://app.coderabbit.ai/settings/billing?tab=usage&orgId=79839cd4-7849-4bd5-8d6d-5ecaf453356e
  https://app.coderabbit.ai/dashboard/review-capacity?orgId=79839cd4-7849-4bd5-8d6d-5ecaf453356e

Build ai-review failed with exit code 1

Decode XecVibe memos with fatal UTF-8 and ignoreBOM.

Harbormaster completed remote builds in B38152: Diff 61322.Thu, Sep 10, 14:09

Build Bitcoin ABC Diffs / Diff Testing (ai-review) failed.

Tail of the build log:

/work /work/abc-ci-builds/ai-review
Connecting to CodeRabbit... 0s elapsed
Preparing review... 2s elapsed

  ✗ Review limit reached

  Limit details: You’ve used all 5 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Check out review usage here.
  You can wait 6 minutes for the limit to reset.
  https://app.coderabbit.ai/settings/billing?tab=usage&orgId=79839cd4-7849-4bd5-8d6d-5ecaf453356e
  https://app.coderabbit.ai/dashboard/review-capacity?orgId=79839cd4-7849-4bd5-8d6d-5ecaf453356e

Build ai-review failed with exit code 1

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) RabbitCop: To debug and serve.

────────────────────────────────────────────────────────────────────────

major [Functional Correctness]
→ ]8;;vscode://file//work/modules/ecash-parse/src/parseTx.ts:665modules/ecash-parse/src/parseTx.ts:665]8;;

Reject bare opcodes before creating the app action.

A bare OP_1 produces stackArray[1] === "51". Line 665 decodes it as
"Q" and creates a valid XecVibe action.

The specification requires an actual pushdata operation. Retain the script
operation type or inspect the original output script before accepting the
memo. Change the fixture at Lines 292-345 to expect an invalid action.

────────────────────────────────────────────────────────────────────────

major [Functional Correctness]
→ ]8;;vscode://file//work/cashtab/src/opreturn/index.ts:249cashtab/src/opreturn/index.ts:249]8;;

Reject bare opcodes before decoding the memo.

getStackArray exposes bare OP_1 as "51". Line 249 decodes that value
as "Q" and marks the payload as valid.

The specification requires a pushdata operation and rejects bare opcodes.
Preserve or inspect the script operation type so that OP_1 is invalid
while pushed byte 01 51 remains valid. Update the fixture at Lines
596-604 accordingly.

────────────────────────────────────────
Review complete
2 findings ✔

Major 2

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

Harbormaster completed remote builds in B38154: Diff 61324.Thu, Sep 10, 14:18

Fabien accepted this revision.Thu, Sep 10, 14:22

This revision is now accepted and ready to land.Thu, Sep 10, 14:22

Rabbit is correct, the switch to stackArray dropped the edge case handling

This revision now requires changes to proceed.Thu, Sep 10, 14:44

Reject bare opcodes as XecVibe memo; keep pushed 0x51 valid.

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Always design for concurrency. Allow for concurrency, and you'll design cleaner interfaces with fewer assumptions.

────────────────────────────────────────
Review complete
No findings ✔

12 files reviewed:

cashtab/src/config/opreturn.ts
cashtab/src/opreturn/fixtures/vectors.js
cashtab/src/opreturn/index.ts
doc/standards/op_return-prefix-guideline.md
doc/standards/xecvibe.md
modules/ecash-parse/src/constants/opreturn.ts
modules/ecash-parse/src/fixtures/xecvFixtures.ts
modules/ecash-parse/src/getTxNotificationMsg.test.ts
modules/ecash-parse/src/getTxNotificationMsg.ts
modules/ecash-parse/src/parseTx.test.ts

... and 2 more files
────────────────────────────────────────

Harbormaster completed remote builds in B38159: Diff 61329.Fri, Sep 11, 04:30

Fabien accepted this revision.Fri, Sep 11, 08:24

This revision is now accepted and ready to land.Fri, Sep 11, 08:24

Closed by commit rABCc66209e47e22: [Cashtab] Parse XecVibe OP_RETURN. · Explain WhyFri, Sep 11, 10:41

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABCc66209e47e22: [Cashtab] Parse XecVibe OP_RETURN.

[Cashtab] Parse XecVibe OP_RETURN
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 61337

cashtab/extension/public/manifest.json

cashtab/package.json

cashtab/public/native-latest.json

cashtab/src/config/opreturn.ts

cashtab/src/opreturn/fixtures/vectors.js

cashtab/src/opreturn/index.ts

doc/standards/op_return-prefix-guideline.md

doc/standards/xecvibe.md

modules/ecash-parse/src/constants/opreturn.ts

modules/ecash-parse/src/fixtures/xecvFixtures.ts

modules/ecash-parse/src/getTxNotificationMsg.test.ts

modules/ecash-parse/src/getTxNotificationMsg.ts

modules/ecash-parse/src/parseTx.test.ts

modules/ecash-parse/src/parseTx.ts

modules/ecash-parse/src/types.ts

[Cashtab] Parse XecVibe OP_RETURNClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 61337

cashtab/extension/public/manifest.json

cashtab/package.json

cashtab/public/native-latest.json

cashtab/src/config/opreturn.ts

cashtab/src/opreturn/fixtures/vectors.js

cashtab/src/opreturn/index.ts

doc/standards/op_return-prefix-guideline.md

doc/standards/xecvibe.md

modules/ecash-parse/src/constants/opreturn.ts

modules/ecash-parse/src/fixtures/xecvFixtures.ts

modules/ecash-parse/src/getTxNotificationMsg.test.ts

modules/ecash-parse/src/getTxNotificationMsg.ts

modules/ecash-parse/src/parseTx.test.ts

modules/ecash-parse/src/parseTx.ts

modules/ecash-parse/src/types.ts

[Cashtab] Parse XecVibe OP_RETURN
ClosedPublic
Actions

Revision Contents
Changeset List