Page MenuHomePhabricator

[Cashtab] Deprecate BCH from parseChronikTx
ClosedPublic

Authored by bytesofman on Dec 6 2022, 13:15.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC17067f345de8: [Cashtab] Deprecate BCH from parseChronikTx
Summary

T2730

After some wallet improvements, we now have a local function that converts hash160 to ecash: address. Implement this in parseChronikTx in place of the BCH method.

Note: A couple of bugs were uncovered here that had been lurking behind the need to mock address conversions using the legacy method.

  1. The app was calculating replyAddress for every input, not just the first input, and thereby using the last one.
  2. The app was mocking and returning the wrong replyAddress for the swap tx unit test

While 1. above could have been fixed in a stacked diff on top of this, it would have required changing the wrong mock to another wrong mock. Not wishing to intentionally introduce an error, fix is included here.

Test Plan

Review changes to parseChronikTx and npm test
Inspect https://explorer.e.cash/tx/2f030de7c8f80a1ecac3645092dd22f0943c34d54cb734e12d7dfda0641fdfcf and confirm it now lines up with its unit test
npm start
Receive an XEC msg transaction
Reply to it

Diff Detail

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

Event Timeline

emack requested changes to this revision.Dec 7 2022, 04:47
emack added a subscriber: emack.

Tested all ok, however upon sending or receiving messages I'm seeing this unsightly uncaught exception in the console. I've checked in prod and it doesn't show up there.

image.png (214×368 px, 29 KB)

This revision now requires changes to proceed.Dec 7 2022, 04:47

patching bug found in review

Tested all ok, however upon sending or receiving messages I'm seeing this unsightly uncaught exception in the console. I've checked in prod and it doesn't show up there.

image.png (214×368 px, 29 KB)

Another good catch. I need to get more serious about using grep -r to make sure these params are really removed everywhere

This revision is now accepted and ready to land.Dec 7 2022, 06:15