[electrum] properly exclude ALP tokens when sending XEC
ClosedPublic
Actions

Authored by PiRK on Fri, Mar 14, 14:20.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC0e9f232bbe91: [electrum] properly exclude ALP tokens when sending XEC

Summary

D17777 actually didn't achieve its goal of making it impossible to accidentaly burn ALP tokens, it only froze the coins in the Coins tab and made it impossible to spend them from there.

We need to filter the coins in the wallet's get_utxos method as well.

Also filter them in CashFusion (both when autofusing the entire wallet and when inputing coins from the address tab)

Depends on D17806

Test Plan

In a wallet that has ALP tokens and SLP tokens, click the Max button in the spend tab, add a destination address, click "Preview" and check that none of the token UTXOs was included in the transaction.

Go to the Address tab, right click on an address that has only ALP tokens (no regular spendable utxos), check that there is no "Input coin to CashFusion" context action

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32742
Build 64972: Build Diff	electrum-tests
Build 64971: arc lint + arc unit

Event Timeline

PiRK created this revision.Fri, Mar 14, 14:20

Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, Mar 14, 14:20

PiRK requested review of this revision.Fri, Mar 14, 14:20

PiRK edited the summary of this revision. (Show Details)Fri, Mar 14, 14:21

PiRK added a parent revision: D17806: [electrum] Make the coin consolidator not burn alp tokens.

Harbormaster completed remote builds in B32731: Diff 53115.Fri, Mar 14, 14:23

PiRK added a child revision: D17809: [electrum] make it possible to spend tokens by manually selecting the coins to spend.Fri, Mar 14, 14:29

As an additional test I'm also going to try to be in a few CashFusion rounds to confirm no tokens are burned. But I cannot guarantee the tests will be conclusive in a timely manner.

Fabien requested changes to this revision.Sun, Mar 16, 19:24

Fabien added a subscriber: Fabien.

Fabien added inline comments.

electrum/electrumabc/wallet.py
1117 ↗	(On Diff #53115)	you should add a `is_token` method that returns `slp_token or is_alp_token` as it's used in several places

This revision now requires changes to proceed.Sun, Mar 16, 19:24

factor the token detection into a wallet.has_tokens(coin) method and fix one more thing that could cause accidental spending of ALP tokens (for users who enabled "Spend only fused coins", if a token is sent to the same address as a fused coin)

Harbormaster completed remote builds in B32742: Diff 53137.Mon, Mar 17, 09:21

PiRK added inline comments.Mon, Mar 17, 09:23

electrum/electrumabc/wallet.py
1090	`has_token` sounds more correct than `is_token` (the utxo has token attached, as well as native XEC coins), so maybe I should also rename `is_alp_token` to `has_alp_token`. Can be done in a separate diff, as `is_alp_token` was not introduced in this diff