- As per T2503, this master diff refactors SendXec() to piece together the new suite of utility functions (D11617, D11626, D11645, D11719, D11710, D11712) to send XEC txs via the chronik client.
- the chronik instance in useWallet is exposed via ContextValue, which is then used by the Send component to pass to sendXec() as an input param.
- getting chronik to play with jest took some effort. There remains three unit tests in useBCH.test.js specific to parseTxData() that are not playing nice but I don't see how they're connected to sendXec() changes. Given my recent environment issues, can you pls uncomment them and run on your end and let me know if you get the same TypeError: process.cwd is not a function error?
- The tx history update lag matter in T2560 is present here but not sure if you want to resolve as part of this or look at it as a separate optimization diff. On a related note, I had to add a false passLoadingStatus statement to the end of send() in Send.js otherwise sending via chronik takes more than 30 seconds before removing the grey loading status thingymabob.
- handleEncryptedOpReturn() is now redundant and will be deprecated in a separate diff.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project
- npm start
- receive XEC tx with no message
- receive XEC tx with message
- receive XEC tx with encrypted message
- send one to one XEC tx with no message
- send one to one XEC tx with message, incl. verify OP_RETURN hex breakdown
i.e.
6a OP_RETURN
04
00746162 _TAB
06
666f6f626172 the msg
- send one to one XEC tx with encrypted message, incl. verify OP_RETURN hex breakdown
i.e.
6a OP_RETURN
04
65746162 ETAB
4c81
042a7f55fb2cebadb6b197025367b8345c94b622b4cc49d34400f956ed8365063c00eed4084839a8ac69c3cc0d7dc25655a0c617c7b805c0fc06d604d88382d85ec31134317c7b9bf1eebefa3b0d237c5ae766eb54b43074120e3ee17c337b1754b1086fa9cac8f250fd4bb0de810e6908b3b5048bb2de7255913adc80c12431be the encrypted msg
- receive one to many XEC tx with no message
- receive one to many XEC tx with message
- send one to many XEC tx with no message
- send one to many XEC tx with message
- send airdrop with no message and verify OP_RETURN hex breakdown
i.e.
6a OP_RETURN
04
64726f70 DROP
20
1c6c9c64d70b285befe733f175d0f384538576876bd280b10587df81279d3f5e token id
04
00746162 _TAB
- send airdrop with message and verify OP_RETURN hex breakdown
i.e.
6a OP_RETURN
04
64726f70 DROP
20
1c6c9c64d70b285befe733f175d0f384538576876bd280b10587df81279d3f5e token id
04
00746162 _TAB
10
61697264726f702077697468206d7367 the msg
- create a new wallet and send/receive a first XEC tx
- activate a saved wallet and send/receive a XEC tx
- ensure all existing sendXec unit tests pass
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- sendXecRefactor
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 19722 Build 39163: Build Diff cashtab-tests Build 39162: arc lint + arc unit
Event Timeline
Comment Actions
OK, good news and bad news.
Good news, I figured out the issue going on with the unit tests.
Bad news, it's another buried async API call that needs to be mocked.
I will create a diff that does this standalone and then we'll rebase on that.
| web/cashtab/src/components/Send/Send.js | ||
|---|---|---|
| 446 ↗ | (On Diff #34406) | Need to dig into this more. The rationale for only including passLoadingStatus(false) in the useEffect loop for when balances.totalBalance changes is this: if a user tries to send another tx before the wallet utxo set has updated (i.e. balances.totalBalance has changed), then they will (often) receive an "invalid input utxos" error -- the wallet will try to create a tx with the utxos that were just used in the previous tx. |
Comment Actions
Rebasing to D11777 fixed the previous error relating to the buried async API call.
However through frontend testing I encountered a change calculation issue introduced by this diff whereby line 1520 in useBCH.js was incorrectly passing in inputUtxos instead of totalInputUtxoValue to reflect the total value of the collated input UTXOs, causing the change amount to become part of the tx fee.
Fixed this by changing line 1520 from txInputObj.inputUtxos to txInputObj.totalInputUtxoValue, however this then broke the unit test suite again with the TypeError: process.cwd is not a function error again.
Currently re-examining the full jest test suite for sendXec.
Comment Actions
Following on from previous comment, post-rebase change calculation bug and subsequent new jest errors now resolved.
| web/cashtab/src/components/Send/Send.js | ||
|---|---|---|
| 446 | We need to figure out what is causing the dealy in balance update and solve that, instead of hardcoding this here to remove the loading spinner on tx completion. As it stands, this patch won't work -- the 'loading' spinner does indeed go away, but the balance does not update, and neither does the utxo set ... so if the user tries to send another tx, it will potentially fail by trying to build with a consumed utxo. UPDATE Okay I did some testing here and figured out what is going on.
A bit of a catch 22 here. On the one hand, it would be nice to implement this tx functionality before we implement the utxo handling -- because dropping in the chronik utxo handling methods will require changing the tx generation functions (the utxo data is accessed at different keys). On the other hand, this type of 30s delay is not acceptable UX. I think the best solution here is
Some alternatives to show why I think the above is the best approach
The 'doublecheck' might not be a bad idea...but it seems kind of dumb to doublecheck chronik utxo changes by asking bch-api again, will always be manually handling the timing differences. So -- let's keep this one ready and I'll build out the chronik utxo handling stacked diff (if you think of a better approach than what I've described above, let me know!) | |
Comment Actions
yup I agree, rebasing this on top of a chronikfied utxo handling logic is the better approach as it's easier to debug the tx generation logic when combined.