Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC5d218549c5d2: [electrum] freeze UTXOs that may contain ALP tokens

Summary

This makes ALP tokens unspendable.

We add a data structure to store outpoints that may be ALP tokens. For now, the detection logic is very basic: we freeze all outputs in any transaction whose first output is a valid EMPP output with at least one payload that starts with the ALP lokad ID.

A known drawback is that this method makes the change output also unspendable (for users who import a seed from a token wallet such as CashTab into Electrum ABC). Future improvements should be to:

make it possible for users to force-spend SLP, ALP and false positive ALP outputs if they insist
properly parse ALP tokens to eliminate these known false positives

Test Plan

Open a wallet that already has an ALP token, make sure that now it is frozen and show as a potential token (green background) in the coins tab.

Send a new alp token to the wallet, make sure it gets frozen and green.

python test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Branch

alp

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32689
Build 64866: Build Diff	electrum-tests
Build 64865: arc lint + arc unit

Event Timeline

PiRK created this revision.Tue, Mar 11, 17:21

Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Mar 11, 17:21

Harbormaster completed remote builds in B32659: Diff 53018.Tue, Mar 11, 17:23

add unittests

PiRK published this revision for review.Tue, Mar 11, 19:56

Harbormaster completed remote builds in B32665: Diff 53024.Tue, Mar 11, 19:57

PiRK edited the summary of this revision. (Show Details)Tue, Mar 11, 20:50

PiRK edited the test plan for this revision. (Show Details)

minor UI improvement (change the greyed out spend action text to tell why the coin is unspendable)

Harbormaster completed remote builds in B32668: Diff 53027.Tue, Mar 11, 21:01

Fabien requested changes to this revision.Wed, Mar 12, 13:24

Fabien added a subscriber: Fabien.

Fabien added inline comments.

electrum/electrumabc/tests/test_alp.py
1 ↗	(On Diff #53027)	The header is missing
electrum/electrumabc/tokens/alp.py
57 ↗	(On Diff #53027)	The comment could be a docstring
80 ↗	(On Diff #53027)	my understanding is that the other outputs can't be tokens or this makes the tx an invalid empp: A valid eMPP tx must have an OP_RETURN output at the first (vout = 0) position. Any OP_RETURN at a position after that should be ignored as invalid.

This revision now requires changes to proceed.Wed, Mar 12, 13:24

PiRK added inline comments.Wed, Mar 12, 16:09

electrum/electrumabc/tokens/alp.py
80 ↗	(On Diff #53027)	My understanding is that op_return in position 0 has the instructions to determine which following outputs (regular p2pkh) should be considered to contain the tokens. The following outputs in a SLP/ALP tx are usually dust outputs that contain just enough XEC to make them spendable (pay the fee), and these are the ones we want to freeze.

Fabien added inline comments.Wed, Mar 12, 16:18

electrum/electrumabc/tokens/alp.py
80 ↗	(On Diff #53027)	Oh ok I missed you were freezing the outputs individually and not all of them all the time. My bad

feedback + handle case of same transaction added twice without clearing first (don't duplicate outpoints, don't even bother rescanning the transaction)

Harbormaster completed remote builds in B32689: Diff 53054.Wed, Mar 12, 18:56

Fabien accepted this revision.Thu, Mar 13, 08:37

This revision is now accepted and ready to land.Thu, Mar 13, 08:37

Closed by commit rABC5d218549c5d2: [electrum] freeze UTXOs that may contain ALP tokens (authored by PiRK). · Explain WhyThu, Mar 13, 10:00

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC5d218549c5d2: [electrum] freeze UTXOs that may contain ALP tokens.

PiRK mentioned this in D17808: [electrum] properly exclude ALP tokens when sending XEC.Fri, Mar 14, 14:20

PiRK mentioned this in rABC0e9f232bbe91: [electrum] properly exclude ALP tokens when sending XEC.Mon, Mar 17, 14:08