[Cashtab][Alias] Patch silent bugs introduced by spec change
ClosedPublic
Actions

Authored by emack on Oct 3 2023, 13:36.

Details

Reviewers

bytesofman

Group Reviewers

Restricted Project

Commits

rABC28e0ad7683c8: [Cashtab][Alias] Patch silent bugs introduced by spec change

Summary

The alias spec redesign a few months ago silently broke a few parseChronikTx related elements. These should be patched together under this single diff as updating the unit test mock will break multiple unit test vectors.

The rendering of the alias name in the tx history tab was no longer being rendered. parseChronikTx was expecting the alias name to be the 2nd array element parsedOpReturnArray[1], which based on the lastest alias spec is now the version number. The alias name is actually the 3rd element in the array now. The unit tests didn't pick this up because the unit test mock data was based on the old spec where the alias name was the 2nd element in the opreturn array. Mock now updated with a live example from explorer.

The live alias registration tx was being incorrectly parsed as incoming by parseChronikTx's unit test. This is because the hash160 mockParseTxWallet didn't match the hash160 from the live example taken from explorer. Added mockParseAliasTxWallet to chronikTxHistory mock. No issue with the code itself.

Test Plan

npm test
npm start
Check tx labelling, icon and amount for the following:

Load a wallet with an existing alias registration tx. Refresh the tx history and ensure label, icon, registration fee and alias name for that tx are displayed correctly.
Send and receive XEC tx.
Send and receive XEC tx with an unencrypted msg.
Send and receive one to many XEC tx.
Send and receive one to many XEC tx with an unencrypted msg.
Send and receive XEC tx with an encrypted msg.
Send and receive an eToken tx
Receive a message from another cashtab wallet.
Receive a message from another ElectrumABC wallet.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

aliasSpecPatch

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25246
Build 50078: Build Diff	cashtab-tests
Build 50077: arc lint + arc unit

Event Timeline

emack created this revision.Oct 3 2023, 13:36

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 3 2023, 13:36

emack requested review of this revision.Oct 3 2023, 13:36

emack edited the summary of this revision. (Show Details)Oct 3 2023, 13:37

Harbormaster completed remote builds in B25246: Diff 42510.Oct 3 2023, 13:43

I'm not understanding why the amount is wrong. The alias registration tx should just be a "send" tx to the IFP address. So the sent amount should be everything that isn't change, like a normal tx.

If this is accepted then I'll update the spec separately to mandate the payment fee as the 2nd output after the first OP_RETURN output.

We don't want to change the spec for this. If a tx comes in paying spec price for an alias using 1000 variable sized utxos that sum to the right amount, that's valid per spec. Should also be how cashtab handles any sent tx.

was showing the total sats associated with ALL outputs rather than just the alias registration fee paid

right this is correct. the OP_RETURN output is 0 satoshis (anyway, even if there are sats there, we shouldn't be adding them). And logic is already in place for not counting change as part of the sent amount.

What was the actual discrepancy you were seeing here?

This revision now requires changes to proceed.Oct 3 2023, 23:00

emack edited the summary of this revision. (Show Details)Oct 4 2023, 10:28

Removed the xecAmount override as it was only needed for the original mock data that was not aligned with the latest alias spec, hence previously returning change. This is no longer the case with the latest mocks. This also removes the previous comment around updating the spec.

Remove newline

Harbormaster completed remote builds in B25258: Diff 42528.Oct 4 2023, 10:32

Harbormaster completed remote builds in B25259: Diff 42529.Oct 4 2023, 10:35