[ecash-herald] Improve formatting of xec send msgs
ClosedPublic
Actions

Authored by bytesofman on Jul 1 2023, 22:20.

Details

Reviewers

PiRK

Group Reviewers

Restricted Project

Commits

rABC15839c6e97c2: [ecash-herald] Improve formatting of xec send msgs

Summary

Format small amounts in fiat if prices are available. Show tx fee as a fiat cost instead of 'satoshis per byte', which is really only interesting to devs.

Showing very small fiat values in fiat is more 'newsworthy' imo. Demonstrates microtransactions and provides actual information, vs satoshis per byte.

Test Plan

npm test

Diff Detail

Repository

rABC Bitcoin ABC

Branch

tighten-xecsend-lines

Lint

Lint Errors

Severity	Location	Code	Message
Error	apps/ecash-herald/src/parse.js:185	ESLINT	no-unused-vars

Unit

No Test Coverage

Build Status

Buildable 24297
Build 48204: Build Diff	ecash-herald-tests
Build 48203: arc lint + arc unit

Event Timeline

bytesofman created this revision.Jul 1 2023, 22:20

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 1 2023, 22:20

bytesofman requested review of this revision.Jul 1 2023, 22:20

Harbormaster completed remote builds in B24297: Diff 41165.Jul 1 2023, 22:21

linting

Harbormaster completed remote builds in B24298: Diff 41166.Jul 1 2023, 22:23

bytesofman added a child revision: D14188: [ecash-herald] Patch sync functions defined as async in utils unit test.Jul 1 2023, 22:24

bytesofman mentioned this in D14180: [ecash-herald] Shorter msg for tx fee desc.Jul 2 2023, 13:05

bytesofman mentioned this in D14181: [ecash-herald] Add space before tx fee for self-send XEC txs to match formatting standard of other XEC txs.Jul 2 2023, 14:03

PiRK added a subscriber: PiRK.Jul 2 2023, 19:10

PiRK added inline comments.

apps/ecash-herald/src/utils.js
172–193 ↗	(On Diff #41166)	nit: This could be simplified to handle the 5 middle cases in one go. But I'm not sure about the performance implications of doing math operations vs 5 tests.

PiRK accepted this revision.Jul 2 2023, 19:11

This revision is now accepted and ready to land.Jul 2 2023, 19:11

bytesofman added inline comments.Jul 2 2023, 22:08

apps/ecash-herald/src/utils.js
172–193 ↗	(On Diff #41166)	also not sure of performance but this is a more elegant approach that is also more robust (what if price of 1 sat needs another decimal place?) using, thanks