Page MenuHomePhabricator

[electrum] properly exclude ALP tokens when sending XEC
ClosedPublic

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

Event Timeline

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

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)

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

This revision is now accepted and ready to land.Mon, Mar 17, 13:03