[ecash-herald] Update parseOpReturn to recognize paywall payments
ClosedPublic
Actions

Authored by emack on Aug 12 2024, 05:11.

Details

Reviewers

bytesofman
Fabien

Group Reviewers

Restricted Project

Commits

Summary

Update the herald to recognize paywall payment txs

Test Plan

npm test

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

emack requested review of this revision.Aug 12 2024, 05:11

Look at your own test, is that really what you would expect to be printed by the herald ? Start with what it should look like first

apps/ecash-herald/src/parse.js
673 ↗	(On Diff #49147)	What is the point of displaying an invalid txid ?
675 ↗	(On Diff #49147)	The txid should be an explorer link, printing the long sequence of chars brings no value and is annoying
678 ↗	(On Diff #49147)	This gives zero information, off what spec ?

This revision now requires changes to proceed.Aug 12 2024, 08:16

Updated article paywall render to be an explorer link, removed invalid article id renders and added reference to spec.

apps/ecash-herald/src/parse.js
673 ↗	(On Diff #49147)	Removed.
675 ↗	(On Diff #49147)	Updated to an explorer link
678 ↗	(On Diff #49147)	Off spec refers to this spec here on the mono repo. I've added this reference as a comment in this block.

apps/ecash-herald/src/parse.js
672 ↗	(On Diff #49149)	...txid would be 64 chars long, no? 2 char per byte, 32 bytes
apps/ecash-herald/test/mocks/appTxSamples.js
585 ↗	(On Diff #49149)	so the txid here is `6494c1ef1a1d5fc399952e218514b0f968d7142305e86719bda196229e28982e` ... but it is encoded as `36343934633165663161316435666333393939353265323138353134623066393638643731343233303565383637313962646131393632323965323839383265` why is the txid not just direct hex encoded here. wasteful to ascii encode hex .. just using twice as many bytes and extra decode step for the hell of it ...am I reading this wrong? beyond the scope of this diff but if that is what is going on, should be fixed

Fabien requested changes to this revision.Aug 12 2024, 17:52

Fabien added inline comments.

apps/ecash-herald/src/parse.js
672 ↗	(On Diff #49149)	Damn, I missed the elephant in the room... good catch
678 ↗	(On Diff #49147)	This is not helpful. On the herald having "on spec" doesn't tell you what spec it attempted to use. "off spec paywall payment" is much more helpful for example.

This revision now requires changes to proceed.Aug 12 2024, 17:52

Updated to parse hex article txids and updated off spec messaging

apps/ecash-herald/test/mocks/appTxSamples.js
585 ↗	(On Diff #49149)	I have corrected this on eCashChat's side so the opreturn script has the article txid in hex form and also updated it to recognize both formats so historical paywall payments are not affected. I figured might as well do it now so we don't have to update herald again later on. Will publish the eCashChat updates to prod first then land this one afterwards.