Page MenuHomePhabricator
Differential D18785

[Cashtab] Rebase NFToa Protocol branches into a unified integration branch
ClosedPublic
Actions

Authored by nftoa on Oct 10 2025, 18:36.

Details

Reviewers
Fabien
bytesofman
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABCdd6ddc57493e: [Cashtab] Rebase NFToa Protocol branches into a unified integration branch
Summary

This branch consolidates all related NFToa Protocol development branches
into a single integration branch for easier testing and review.

Hierarchy:

master (latest)
  └── D18757 [Cashtab] About NFToa Protocol ID (rebase & rename branch)
        └── D18656 [Cashtab] Claim NFToa Protocol ID (rebase & rename branch)
              └── D18784 [Cashtab] Setup NFToa Icon (rebase)
                    └── D18785 [Cashtab] NFToa Protocol ID Integration (rebase & rename branch)

Purpose:

• Ensure all related features are aligned with the latest master.
• Simplify future landing by maintaining a clean rebase hierarchy.
• Make review and testing of the entire NFToa stack easier.
Test Plan
  1. See demo https://x.com/nftoa_/status/1979307380056953317
  2. Read documentation doc/standards/nftoa.md
  3. npm test

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

nftoa created this revision.Oct 10 2025, 18:36
Owners added a reviewer: Restricted Owners Package.Oct 10 2025, 18:36
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 10 2025, 18:36
nftoa requested review of this revision.Oct 10 2025, 18:36
Harbormaster completed remote builds in B34756: Diff 56136.Oct 10 2025, 18:42
nftoa retitled this revision from NFToa Protocol ID Integration to [Cashtab] NFToa Protocol ID Integration.Oct 10 2025, 18:43
nftoa edited the summary of this revision. (Show Details)
nftoa edited the test plan for this revision. (Show Details)
nftoa added reviewers: Fabien, bytesofman.
nftoa added subscribers: nftoa, Fabien, bytesofman.
bytesofman requested changes to this revision.Oct 12 2025, 12:02

modified functions should have tests covering the new case, i.e.

opreturn/__tests__/index.test.js
chronik/__tests__/index.test.ts
components/Home/Tx/__tests__/Tx.test.ts

will need to add one or two example txs to chronik/fixtures/mocks.js, then update test vectors with those txs

This revision now requires changes to proceed.Oct 12 2025, 12:02
nftoa mentioned this in D18757: [Cashtab] About NFToa Protocol ID.Oct 13 2025, 18:34
nftoa updated this revision to Diff 56182.Wed, Oct 15, 17:40

[Cashtab] Add unit tests and adjust for NFToa Protocol ID behavior

Harbormaster completed remote builds in B34787: Diff 56182.Wed, Oct 15, 17:46
nftoa edited the test plan for this revision. (Show Details)Wed, Oct 15, 18:05
bytesofman requested changes to this revision.Thu, Oct 16, 00:52

have you run this locally? can you include some screenshots of what these txs look like in history?

cashtab/src/opreturn/__tests__/index.test.js
62 ↗(On Diff #56182)

which is it?

just test for deep equal match to parsed

AI has a habit of writing weird tests like this that aren't really telling us the expected behavior

63 ↗(On Diff #56182)

what's the point of this check?

just do expect(parsed).to.deepEqual(<whatItLooksLike>)

This revision now requires changes to proceed.Thu, Oct 16, 00:52
nftoa updated this revision to Diff 56187.Thu, Oct 16, 06:10
nftoa marked 2 inline comments as done.

Remove ambiguous and unused test function

Harbormaster completed remote builds in B34792: Diff 56187.Thu, Oct 16, 06:15
bytesofman requested changes to this revision.Fri, Oct 17, 16:31

have you run this locally? can you include some screenshots of what these txs look like in history?

This revision now requires changes to proceed.Fri, Oct 17, 16:31
nftoa edited the summary of this revision. (Show Details)Fri, Oct 17, 22:06
nftoa requested review of this revision.Fri, Oct 17, 22:10

Added a demonstration video showcasing how the NFToa Protocol integration works in action on localhost, covering both Authentication Transaction and Regular Message Transaction as defined in the official NFToa documentation.

A demo link has been included in the revision summary for reviewer reference.

cashtab/src/opreturn/__tests__/index.test.js
62 ↗(On Diff #56182)

I would like to apologize for using AI assistance to help write unit tests for my protocol, and for accepting AI-generated suggestions without fully understanding their implications. I understand your concerns, and in the next set of changes, I will remove this part. Thank you for your feedback.

bytesofman accepted this revision.Sat, Oct 18, 03:31
bytesofman added inline comments.
cashtab/src/opreturn/__tests__/index.test.js
62 ↗(On Diff #56182)

using AI to generate tests is fine, but it's important to understand what the tests are doing, esp if they are testing what you are looking to test

using AI is not a red flag. But using AI to generate code you do not understand is not good practice and is will cause all kinds of problems; both for you and your users.

This revision is now accepted and ready to land.Sat, Oct 18, 03:31
bytesofman added a comment.Sat, Oct 18, 03:35

arcpatch-D18785

and D18757 is branch arcpatch-D18757_2

how did you get these branch names?

general workflow can be found at https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/CONTRIBUTING.md

image.png (343×567 px, 34 KB)

it's an issue bc it looks like the branches may not be on master, or the latest master, which could cause problems when you try to land them.

nftoa planned changes to this revision.EditedSat, Oct 18, 05:16
In D18785#428694, @bytesofman wrote:

arcpatch-D18785

and D18757 is branch arcpatch-D18757_2

how did you get these branch names?

general workflow can be found at https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/CONTRIBUTING.md

image.png (343×567 px, 34 KB)

it's an issue bc it looks like the branches may not be on master, or the latest master, which could cause problems when you try to land them.

The branch names like arcpatch-D18757_2 were automatically generated by arc patch.

To give some context, I accidentally deleted my fork of bitcoin-abc earlier when I was still learning how to use Phabricator and adapt to the project workflow. Later, I re-forked the repository and continued working on my last open revision by running arc patch D18757. That command automatically created the arcpatch-D18757_2 branch based on the revision ID.

I understand that these branches might not be based on the latest master, so I’ll make sure to rebase them properly before updating or landing the diff.

nftoa updated this revision to Diff 56201.Sat, Oct 18, 07:08

[Cashtab] Merge NFToa Protocol branches into a single integration branch

This revision is now accepted and ready to land.Sat, Oct 18, 07:08
Harbormaster completed remote builds in B34800: Diff 56201.Sat, Oct 18, 07:14
nftoa retitled this revision from [Cashtab] NFToa Protocol ID Integration to [Cashtab] Merge NFToa Protocol branches into a single integration branch.Sat, Oct 18, 07:16
nftoa edited the summary of this revision. (Show Details)
nftoa edited the test plan for this revision. (Show Details)
nftoa retitled this revision from [Cashtab] Merge NFToa Protocol branches into a single integration branch to [Cashtab] Rebase NFToa Protocol branches into a unified integration branch.Sat, Oct 18, 12:07
nftoa edited the summary of this revision. (Show Details)
nftoa updated this revision to Diff 56205.Sat, Oct 18, 12:20

[Cashtab] Rebase NFToa Protocol branches into a unified integration branch

Harbormaster completed remote builds in B34804: Diff 56205.Sat, Oct 18, 12:26
nftoa updated this revision to Diff 56210.Sat, Oct 18, 14:22

fix author n rebase

Harbormaster completed remote builds in B34809: Diff 56210.Sat, Oct 18, 14:28
nftoa mentioned this in rABC957d4368fff8: [Cashtab] About NFToa Protocol ID.Mon, Oct 20, 13:06
nftoa updated this revision to Diff 56234.Mon, Oct 20, 14:51

apply D18785 diff raw & rebase

This revision was landed with ongoing or failed builds.Mon, Oct 20, 14:52
Closed by commit rABCdd6ddc57493e: [Cashtab] Rebase NFToa Protocol branches into a unified integration branch (authored by nftoa). · Explain Why
This revision was automatically updated to reflect the committed changes.
nftoa added a commit: rABCdd6ddc57493e: [Cashtab] Rebase NFToa Protocol branches into a unified integration branch.
Harbormaster completed remote builds in B34826: Diff 56234.Mon, Oct 20, 15:21

Revision Contents
Changeset List

PathSize
cashtab/
src/
chronik/
fixtures/
178 lines
48 lines
42 lines
components/
Home/
Tx/
__tests__/
114 lines
28 lines
config/
1 line
opreturn/
__tests__/
40 lines
fixtures/
67 lines
20 lines

Diff 56235

View Options

cashtab/src/chronik/fixtures/mocks.js

Loading...
View Options

cashtab/src/chronik/fixtures/vectors.js

Loading...
View Options

cashtab/src/chronik/index.ts

Loading...
View Options

cashtab/src/components/Home/Tx/__tests__/index.test.js

Loading...
View Options

cashtab/src/components/Home/Tx/index.tsx

Loading...
View Options

cashtab/src/config/opreturn.ts

Loading...
View Options

cashtab/src/opreturn/__tests__/index.test.js

Loading...
View Options

cashtab/src/opreturn/fixtures/vectors.js

Loading...
View Options

cashtab/src/opreturn/index.ts

Loading...