Page MenuHomePhabricator
Differential D18505

[cashtab] Improve PayButton user experience
ClosedPublic
Actions

Authored by Fabien on Aug 18 2025, 21:56.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC905accce6b98: [cashtab] Improve PayButton user experience
Summary

By broadcasting the to appropriated node so the notifaction comes with no delay.

Test Plan
./contrib/teamcity/build-configurations.py cashtab-tests

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cashtab_paybutton
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 34182
Build 67832: Build Diffcashtab-tests
Build 67831: arc lint + arc unit

Event Timeline

Fabien created this revision.Aug 18 2025, 21:56
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 18 2025, 21:56
Fabien added a comment.Aug 18 2025, 21:56

@bot preview-cashtab

teamcity awarded a token.Aug 18 2025, 21:56
teamcity added a comment.Aug 18 2025, 21:59

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.68.37.192:41831 for the next 60 minutes.

Harbormaster completed remote builds in B34182: Diff 55288.Aug 18 2025, 22:00
Fabien updated this revision to Diff 55293.Aug 18 2025, 22:31

Add log

Fabien added a comment.Aug 18 2025, 22:52

@bot preview-cashtab

teamcity updated a token.Aug 18 2025, 22:52
Harbormaster completed remote builds in B34184: Diff 55293.Aug 18 2025, 22:53
teamcity added a comment.Aug 18 2025, 22:55

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.83.220.63:41763 for the next 60 minutes.

Fabien updated this revision to Diff 55298.Aug 19 2025, 08:05

Rebase, fix and more logs

Harbormaster completed remote builds in B34188: Diff 55298.Aug 19 2025, 08:12
Klakurka added a subscriber: Klakurka.Aug 19 2025, 16:24

@bot preview-cashtab

teamcity updated a token.Aug 19 2025, 16:24
teamcity added a comment.Aug 19 2025, 16:27

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.91.14.25:41120 for the next 60 minutes.

Klakurka added a comment.Aug 19 2025, 22:01

@bot preview-cashtab

teamcity updated a token.Aug 19 2025, 22:01

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.91.14.25:41778 for the next 60 minutes.

Klakurka added a comment.Aug 20 2025, 01:43

@bot preview-cashtab

teamcity updated a token.Aug 20 2025, 01:43

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.91.14.25:41082 for the next 60 minutes.

Klakurka added a comment.Aug 20 2025, 23:55

@bot preview-cashtab

teamcity updated a token.Aug 20 2025, 23:55

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.91.14.25:41985 for the next 60 minutes.

Klakurka added a comment.Aug 21 2025, 00:07

We're still working on the CORS error but upon PB chronik msg failure, we should still continue to clear out the Send inputs like normal. Also don't bother throwing an error - it just spooks people and PB doesn't necessarily need our own node online for stuff to work since we might be doing an upgrade or something, falling back to other public nodes.

Klakurka added a comment.Aug 21 2025, 00:20

Good news - we got the CORS issue sorted out so that's working now.

Bad news - no latency improvement in my testing.

Good news (2) - our chronik node is detecting the tx very quickly (0.1-0.2s) so we're going to take a look at what's going on there and report back.

Klakurka added a comment.Aug 21 2025, 22:17

@bot preview-cashtab

teamcity updated a token.Aug 21 2025, 22:17
teamcity added a comment.Aug 21 2025, 22:20

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.89.41.69:41583 for the next 60 minutes.

Fabien added a comment.Aug 22 2025, 08:25
In D18505#422066, @Klakurka wrote:

We're still working on the CORS error but upon PB chronik msg failure, we should still continue to clear out the Send inputs like normal. Also don't bother throwing an error - it just spooks people and PB doesn't necessarily need our own node online for stuff to work since we might be doing an upgrade or something, falling back to other public nodes.

The code doesn't show any error if it fails. It's just trying but since it's not mandatory for proper operation the result is ignored. It's also normal if it fails, it might just know about the tx already due to other nodes relaying it.

Klakurka added a comment.Aug 22 2025, 13:42
In D18505#422217, @Fabien wrote:
In D18505#422066, @Klakurka wrote:

We're still working on the CORS error but upon PB chronik msg failure, we should still continue to clear out the Send inputs like normal. Also don't bother throwing an error - it just spooks people and PB doesn't necessarily need our own node online for stuff to work since we might be doing an upgrade or something, falling back to other public nodes.

The code doesn't show any error if it fails. It's just trying but since it's not mandatory for proper operation the result is ignored. It's also normal if it fails, it might just know about the tx already due to other nodes relaying it.

There's a red error toast in the UI for me when it happens.

Klakurka added a comment.Aug 22 2025, 14:40

@bot preview-cashtab

teamcity updated a token.Aug 22 2025, 14:41

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.91.14.25:41879 for the next 60 minutes.

Fabien added a comment.Aug 22 2025, 15:17
In D18505#422218, @Klakurka wrote:
In D18505#422217, @Fabien wrote:
In D18505#422066, @Klakurka wrote:

We're still working on the CORS error but upon PB chronik msg failure, we should still continue to clear out the Send inputs like normal. Also don't bother throwing an error - it just spooks people and PB doesn't necessarily need our own node online for stuff to work since we might be doing an upgrade or something, falling back to other public nodes.

The code doesn't show any error if it fails. It's just trying but since it's not mandatory for proper operation the result is ignored. It's also normal if it fails, it might just know about the tx already due to other nodes relaying it.

There's a red error toast in the UI for me when it happens.

Oh OK, can you paste the error ? I will catch it

Klakurka added a comment.Aug 22 2025, 16:04
In D18505#422222, @Fabien wrote:
In D18505#422218, @Klakurka wrote:
In D18505#422217, @Fabien wrote:
In D18505#422066, @Klakurka wrote:

We're still working on the CORS error but upon PB chronik msg failure, we should still continue to clear out the Send inputs like normal. Also don't bother throwing an error - it just spooks people and PB doesn't necessarily need our own node online for stuff to work since we might be doing an upgrade or something, falling back to other public nodes.

The code doesn't show any error if it fails. It's just trying but since it's not mandatory for proper operation the result is ignored. It's also normal if it fails, it might just know about the tx already due to other nodes relaying it.

There's a red error toast in the UI for me when it happens.

Oh OK, can you paste the error ? I will catch it

Now that we fixed the cors issues... I'm not sure how to get it to fail on our end to grab a screenshot but maybe you can try updating the URL to something totally invalid and seeing if that displays the error toast. I don't remember the exact error message but it's something along the lines of being "unable to connect to known nodes".

Good news though - my test issue yesterday might have been some weird caching because we're able to confirm that speed is great now. For me, it cut the latency in half.

Assuming we decide to go ahead with our direct-to-chronik paybutton branch (on our end), we're going to make sure we're prioritizing our own node rather than using the "Closest" strategy.

If we can get that error toast sorted out, this LGTM.

Fabien updated this revision to Diff 55337.Aug 25 2025, 07:56

Remove all debug logs, prevent the toast error

Harbormaster completed remote builds in B34215: Diff 55337.Aug 25 2025, 08:03
Fabien added a comment.Aug 25 2025, 08:10

Note that there is no test for this feature, for a couple reasons:

  • It would require a whole new mock and whiteboxing refactor to do so, which seems overkill
  • This feature can fail with no impact on the user, only a small additional delay (maybe 1s to 2s)
Fabien published this revision for review.Aug 25 2025, 12:54
bytesofman added a subscriber: bytesofman.Aug 26 2025, 17:54
bytesofman added inline comments.
cashtab/src/transactions/index.js
208 ↗(On Diff #55337)

won't we already have broadcast the tx here?

217 ↗(On Diff #55337)

so, could expect this to fail if the paybutton server already saw it

silently handled but seems like an unnecessary delay, shouldn't an isPaybutton tx only try to broadcast to paybutton, mb if that fails, try await chronik.broadcastTx(hex, isBurn);

Fabien added inline comments.Aug 26 2025, 18:05
cashtab/src/transactions/index.js
208 ↗(On Diff #55337)

Yes we do, the code adds an extra broadcast. It's a best effort for sharing the tx with the paybutton node directly in order to reduce the latency.

217 ↗(On Diff #55337)

I think we always want the normal broadcast to infra nodes that we support. This ensure the proper behavior, while the extra broadcast is only an latency optimization. If the optimization fails the paybutton will work just fine, it might just take a bit longer to give feedback.

bytesofman accepted this revision.Aug 26 2025, 18:13
This revision is now accepted and ready to land.Aug 26 2025, 18:13
Closed by commit rABC905accce6b98: [cashtab] Improve PayButton user experience (authored by Fabien). · Explain WhyAug 26 2025, 18:15
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC905accce6b98: [cashtab] Improve PayButton user experience.

Revision Contents
Changeset List

PathSize
cashtab/
src/
components/
Send/
7 lines
transactions/
11 lines

Diff 55288

View Options

cashtab/src/components/Send/SendXec.tsx

Loading...
View Options

cashtab/src/transactions/index.js

Loading...