Page MenuHomePhabricator
Differential D15319

[Cashtab] Parse PayButton txs in tx history
ClosedPublic
Actions

Authored by bytesofman on Jan 29 2024, 18:14.

Details

Reviewers
Klakurka
Fabien
Group Reviewers
Restricted Project
Commits
rABC7ac2848ddcda: [Cashtab] Parse PayButton txs in tx history
Summary

T3423

Parse PayButton txs in tx history

Note: this will still render such txs as 'external msgs', but they will at least be legible. Will handle the msg titling issue in a separate diff, as parseChronikTx should be refactored to give an app name instead of a series of app flags.

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cashtab-parse-pb-txs
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 26752
Build 53072: Build Diffcashtab-tests
Build 53071: arc lint + arc unit

Event Timeline

bytesofman created this revision.Jan 29 2024, 18:14
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 29 2024, 18:14
Harbormaster completed remote builds in B26728: Diff 44696.Jan 29 2024, 18:17
bytesofman updated this revision to Diff 44697.Jan 29 2024, 18:20

cover untested cases

bytesofman published this revision for review.Jan 29 2024, 18:21
Harbormaster completed remote builds in B26729: Diff 44697.Jan 29 2024, 18:23
Klakurka added a subscriber: Klakurka.Jan 29 2024, 19:32

Can you put this on the dev URL?

bytesofman added a comment.Jan 29 2024, 19:36
In D15319#344322, @Klakurka wrote:

Can you put this on the dev URL?

now live at https://cashtab-local-dev.netlify.app

Klakurka requested changes to this revision.Jan 30 2024, 00:56
Klakurka added inline comments.
cashtab/src/chronik/index.js
662 ↗(On Diff #44697)

I think it's worth creating a new task to better control the behaviour of the entire "payment details" div. An example of what we could eventually do:

image.png (208×700 px, 20 KB)

Let me know what you think.

For the purposes of getting something simple working, how about:

image.png (241×685 px, 28 KB)

If the payment ID is missing, omit it entirely.

If the custom data section is missing, omit it entirely.

This revision now requires changes to proceed.Jan 30 2024, 00:56
bytesofman added inline comments.Jan 30 2024, 03:58
cashtab/src/chronik/index.js
662 ↗(On Diff #44697)

I think it's worth creating a new task to better control the behaviour of the entire "payment details" div. An example of what we could eventually do:

yes. need to refactor this parsing function first to get it to recognize each app tx. but then pretty straightforward from there.

If the payment ID is missing, omit it entirely.

done

If the custom data section is missing, omit it entirely.

done

bytesofman updated this revision to Diff 44705.Jan 30 2024, 03:58

if no data or no payment id, do not render

bytesofman added a comment.Jan 30 2024, 04:00
In D15319#344364, @bytesofman wrote:

if no data or no payment id, do not render

This is now live at https://cashtab-local-dev.netlify.app

Harbormaster completed remote builds in B26737: Diff 44705.Jan 30 2024, 04:02
Fabien requested changes to this revision.Jan 30 2024, 09:20
Fabien added a subscriber: Fabien.
Fabien added inline comments.
cashtab/src/chronik/fixtures/vectors.js
153 ↗(On Diff #44705)

This has a payment id

158 ↗(On Diff #44705)

There is no test for no nonce but with data. Is that only a possible valid combination ?

cashtab/src/chronik/index.js
643 ↗(On Diff #44705)

remove

651–657 ↗(On Diff #44705)
This revision now requires changes to proceed.Jan 30 2024, 09:20
bytesofman updated this revision to Diff 44723.Jan 30 2024, 14:33

remove debug log, add mocks, support opReturnMessage as partial string in parseChronikTx

bytesofman marked 4 inline comments as done.Jan 30 2024, 14:36
bytesofman updated this revision to Diff 44724.Jan 30 2024, 14:36

remove if/else pattern and use break

Harbormaster completed remote builds in B26746: Diff 44723.Jan 30 2024, 14:38
Harbormaster completed remote builds in B26747: Diff 44724.Jan 30 2024, 14:40
Fabien requested changes to this revision.Jan 30 2024, 15:26
Fabien added inline comments.
cashtab/src/chronik/index.js
609 ↗(On Diff #44724)
613 ↗(On Diff #44724)

This is unrelated to this diff

This revision now requires changes to proceed.Jan 30 2024, 15:26
bytesofman updated this revision to Diff 44730.Jan 30 2024, 15:36

toString everything did not need to be fixed here

Harbormaster completed remote builds in B26752: Diff 44730.Jan 30 2024, 15:40
Fabien accepted this revision.Jan 30 2024, 15:43
Klakurka requested changes to this revision.Jan 30 2024, 18:27

I'm not seeing any of the custom data anymore in the latest commit -- it was working in the first one. Only the payment ID is coming through.

This revision now requires changes to proceed.Jan 30 2024, 18:27
Klakurka added a comment.Jan 30 2024, 18:34

https://reviews.bitcoinabc.org/T3428

bytesofman added a comment.Jan 30 2024, 19:50
In D15319#344664, @Klakurka wrote:

I'm not seeing any of the custom data anymore in the latest commit -- it was working in the first one. Only the payment ID is coming through.

can you give some example txids?

Klakurka added a comment.EditedJan 31 2024, 07:22
In D15319#344671, @bytesofman wrote:
In D15319#344664, @Klakurka wrote:

I'm not seeing any of the custom data anymore in the latest commit -- it was working in the first one. Only the payment ID is coming through.

can you give some example txids?

They're all the same for me (only showing the payment id) but here's one:

a2f68195404f5cf8113d75a0677bc32a0db38919ba240141ae639ea5fbeb9644

bytesofman requested review of this revision.Jan 31 2024, 11:23
In D15319#344833, @Klakurka wrote:
In D15319#344671, @bytesofman wrote:
In D15319#344664, @Klakurka wrote:

I'm not seeing any of the custom data anymore in the latest commit -- it was working in the first one. Only the payment ID is coming through.

can you give some example txids?

They're all the same for me (only showing the payment id) but here's one:

a2f68195404f5cf8113d75a0677bc32a0db38919ba240141ae639ea5fbeb9644

this tx only has a payment id and no data ... so this is expected behavior right?

(only showing the payment id)

6a0450415900000008beabd2394024aaaf
6a is op return
04 + 50415900
00 op 0 for version push
00 op 0 in data push means no data
08 + beabd2394024aaaf is payment id

bytesofman updated this revision to Diff 44778.Jan 31 2024, 11:23

rebase

Harbormaster completed remote builds in B26785: Diff 44778.Jan 31 2024, 11:49
Klakurka accepted this revision.Feb 3 2024, 21:07

Nevermind -- looks good.

This revision is now accepted and ready to land.Feb 3 2024, 21:07
Closed by commit rABC7ac2848ddcda: [Cashtab] Parse PayButton txs in tx history (authored by bytesofman). · Explain WhyFeb 4 2024, 00:54
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC7ac2848ddcda: [Cashtab] Parse PayButton txs in tx history.

Revision Contents
Changeset List

PathSize
cashtab/
2 lines
src/
chronik/
fixtures/
206 lines
53 lines
34 lines
config/
1 line

Diff 44730

View Options

cashtab/package.json

Loading...
View Options

cashtab/src/chronik/fixtures/mocks.js

Loading...
View Options

cashtab/src/chronik/fixtures/vectors.js

Loading...
View Options

cashtab/src/chronik/index.js

Loading...
View Options

cashtab/src/config/opreturn.js

Loading...