Page MenuHomePhabricator

[Cashtab] [p3 match wif to address] Refactor sendToken()s XEC utxo collation and signing approach
AbandonedPublic

Authored by emack on Aug 18 2022, 04:03.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary

Depends on D11871

As per T2599, this is part of a stacked diff to sign utxos for non-eToken txs by matching wif to address.
This diff:

  1. Refactors the XEC utxo collation approach from "finding biggest non-slp UTXO" to the standardized "collate enough XEC UTXOs to pay for the tx".
  2. Aligns the ECPair and signing approach via signXecUtxosByAddress. Note: I couldn't separate this into a separate diff because the ECPair is no longer extracted from the largestBchUtxo.
Test Plan

npm test
send an eToken and verify sender and receiver eToken balances are updated and that txFee, XEC change and token change are correct

Diff Detail

Repository
rABC Bitcoin ABC
Branch
sendXecMatchWifToAddr
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19872
Build 39458: Build Diffcashtab-tests
Build 39457: arc lint + arc unit

Event Timeline

emack requested review of this revision.Aug 18 2022, 04:03
bytesofman added inline comments.
web/cashtab/src/hooks/useBCH.js
1045 ↗(On Diff #34701)

why this *2 factor?

1133 ↗(On Diff #34701)

This is basically repeating the signXecUtxosByAddress function

Are we not able to pass inputUtxos and tokenUtxos in one array to this function?

This revision now requires changes to proceed.Aug 18 2022, 15:54
emack marked an inline comment as done.Aug 19 2022, 05:30
emack added inline comments.
web/cashtab/src/hooks/useBCH.js
1045 ↗(On Diff #34701)

one is for the dust representing the eTokens being sent and another one representing the eToken change to the sender

1133 ↗(On Diff #34701)

should be able to, though may need to rename the function to signUtxosByAddress if it's now taking in both xec and etoken UTXOs. Will push an update on this.

emack marked an inline comment as done.

Combined XEC and eToken input UTXOs and passed in together to the renamed signUtxosByAddress() for ECPair matching and utxo signing.

Fixed existing bug where the XEC input utxo collation was only calculating one etokenSats whereas the remainder calculation was using two etokenSats.
Added comments to explain the *2 multiplier to etokenSats

bytesofman added inline comments.
web/cashtab/src/hooks/useBCH.js
1044 ↗(On Diff #34716)

remainder should be defined before the above for loop (as just let remainder), and then defined before the if block that breaks the for loop if enough utxos have been collected.

This way, we do not need to repeat this definition. The if check in the above for loop should just be if (remainder.gte(0)) {break}

1121 ↗(On Diff #34716)

I haven't tested this but I suppose it would work. remainder is a BigNumber here, so let's stick with BigNumber methods.

https://mikemcl.github.io/bignumber.js/

i.e. remainder.toNumber()

This revision now requires changes to proceed.Aug 22 2022, 22:34
emack marked 2 inline comments as done.

Optimized utxo collation for loop and number format

bytesofman added inline comments.
web/cashtab/src/hooks/useBCH.js
1002

We should not have both wallet and slpBalancesAndUtxos as input parameters, since wallet includes the latest slpBalancesAndUtxos

This is an artifact from when the wallet object did not include utxo state information.

Please address this in another part of this stacked diff.

This revision is now accepted and ready to land.Aug 29 2022, 17:40

Due to mixed arcanist version issues, this stack landed in full when p1 landed

Please abandon

This revision now requires changes to proceed.Sep 20 2022, 21:26