Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion failed when calling sendredeem or sendupdate on imported wallet #381

Closed
buffrr opened this issue Mar 2, 2020 · 15 comments · Fixed by #387
Closed

Assertion failed when calling sendredeem or sendupdate on imported wallet #381

buffrr opened this issue Mar 2, 2020 · 15 comments · Fixed by #387

Comments

@buffrr
Copy link
Contributor

buffrr commented Mar 2, 2020

I'm getting Assertion failed when I try to call sendredeem on lost bids or when calling sendupdate on a domain that I won after importing wallet.

Steps to reproduce:

  1. Install a fresh hsd node & run it.
  2. Import wallet with mkwallet using mnemonic phrase.
  3. Attempt to redeem lost bids using sendredeem or call sendupdate to update any record on a TLD.

Error message: Assertion failed

Both commands show assertion failed. Following suggestion by @pinheadmz I replaced blgr in hsd with the master branch to get stack traces.

Logs output:

Assertion failed.
   at Balance.applyTo (/home/aa/hsd/lib/wallet/txdb.js:3123:5)
   at TXDB.updateAccountBalance (/home/aa/hsd/lib/wallet/txdb.js:214:11)
   at <anonymous>
@pinheadmz
Copy link
Member

Did you rescan after importing the mnemonic phrase? Did you make bids from an different wallet / node?

@buffrr
Copy link
Contributor Author

buffrr commented Mar 2, 2020

I did call rescan 0 after importing it. The wallet already had existing bids from a different node. I didn't make bids from other wallets. This was the only wallet used.

@mslipper
Copy link
Contributor

mslipper commented Mar 7, 2020

When viewing this wallet's balances, there are a bunch of locked coins. For example:

{
  "network": "main",
  "wid": 1,
  "id": "allison",
  "watchOnly": false,
  "accountDepth": 1,
  "token": "",
  "tokenDepth": 0,
  "master": {
  },
  "balance": {
    "account": -1,
    "tx": 16,
    "coin": 15,
    "unconfirmed": 100000,
    "confirmed": 100000,
    "lockedUnconfirmed": 0,
    "lockedConfirmed": 10000
  }
}

The line in HSD that is failing is this one:

https://github.com/handshake-org/hsd/blob/master/lib/wallet/txdb.js#L3123

This leads me to believe that either the wrong set of outputs are being selected, or the applyTo algorithm is wrong.

Both cases I've seen have had enough money to pay for the update.

@mslipper
Copy link
Contributor

mslipper commented Mar 7, 2020

Also - these wallets have been rescanned from zero and nonces have been imported.

@mslipper
Copy link
Contributor

mslipper commented Mar 7, 2020

I did some further digging, and had a hacked HSD node output the value of delta and balance in the TXDB.updateAccountBalance method. Here's the output:

[info] (wallet) Incoming transaction for 1 wallets in WalletDB (0cb3bbd8f613916a81205c09adc904a21dd1fa000a42d06179f27cc0afe62b5d).
delta is <Balance tx=1 coin=1 unconfirmed=-0.0231 confirmed=0.0 lockedUnconfirmed=-20.0 lockedConfirmed=0.0>
balance is <Balance tx=76 coin=13 unconfirmed=306.994987 confirmed=306.994987 lockedUnconfirmed=0.0 lockedConfirmed=305.0>
[error] (node) Assertion failed.

Basically, this means that no user will be able to send updates if they have a zero lockedUnconfirmed balance.

@mslipper
Copy link
Contributor

mslipper commented Mar 7, 2020

Continuing to update here - a workaround is to place a bid on another name, which will increase your lockedUnconfirmed balance. Once this is done, you can then send the update.

@dylanbathurst
Copy link

After a bunch of attempts, it seems as though you need to place enough in bids so that lockedUnconfirmed is greater than the locked up amount on the tld you're trying to reveal. Great find, @mslipper 🙌

@pinheadmz
Copy link
Member

Can we also confirm that all cases of this issue occurred in a rescanned wallet with imported nonces?

@mslipper
Copy link
Contributor

mslipper commented Mar 7, 2020

@pinheadmz this is confirmed, it only occurs when wallets are rescanned with imported nonces.

Here's a script that will reproduce the issue 100% of the time on regtest:

#!/usr/bin/env bash

set -e

hsw-cli --network=regtest --mnemonic="panda day strike tomato rapid inch market box student shell stool evolve middle brush fork news planet fiscal city range chuckle monitor veteran address" mkwallet main
hsw-cli --network=regtest --mnemonic="trust estate winter coil nut deal few vast tooth viable mango virus park garage ocean chapter collect broom shallow soap fox donate misery modify" mkwallet alt
mine_to_addr=`hsw-cli --network=regtest --id=alt account get default | jq -r .receiveAddress`

mine () {
    hsd-cli rpc --network=regtest generatetoaddress $1 $mine_to_addr
}

mine 10
send_addr=`hsw-cli --id=main --network=regtest account get default | jq -r .receiveAddress`
hsw-cli --id=alt --network=regtest --value=100 --address=$send_addr send
mine 1

hsw-rpc --network=regtest selectwallet main
hsw-rpc --network=regtest sendopen foobar
mine 7
hsw-rpc --network=regtest sendbid foobar 1 10
mine 5

read -p "Stop HSD now. Press enter when finished..."
rm -rf ~/.hsd/regtest/wallet/*
read -p "Start HSD now. Press enter when finished..."
hsw-cli --network=regtest --mnemonic="panda day strike tomato rapid inch market box student shell stool evolve middle brush fork news planet fiscal city range chuckle monitor veteran address" mkwallet main
hsw-cli --network=regtest --mnemonic="trust estate winter coil nut deal few vast tooth viable mango virus park garage ocean chapter collect broom shallow soap fox donate misery modify" mkwallet alt
hsw-cli --id=main --network=regtest rescan 0
echo "Rescanning..."
sleep 10

bid_addr=`hsw-cli --network=regtest --id=main history | jq -r '[ .[].outputs[] ] | .[] | select(.covenant.action == "BID") | .address'`
hsw-rpc --network=regtest selectwallet main
hsw-rpc --network=regtest importnonce foobar $bid_addr 1

echo "Rescanning again..."
hsw-cli --id=main --network=regtest rescan 0
sleep 10

echo ""
echo "###########################"
echo "# Here come the problems! #"
echo "###########################"
echo ""

echo "Printing out the account balance:"
hsw-cli --network=regtest --id=main account get default
echo "Observe that lockedUnconfirmed is zero in the above output."

echo "Attempting to send reveal..."
hsw-rpc --network=regtest --id=main sendreveal foobar || 0
echo "Note the failed assertion."
echo "We know that the nonce was properly inserted because the"
echo "error message is a failed assertion rather than \"Blind"
echo "value not found.\""

You'll need two terminals, the hsw-cli toolchain, and jq to run this. Make sure to wipe your regtest data directory before running it. Start hsd in terminal 1, then run this script in terminal 2. Start/stop hsd in terminal 1 when the script tells you to.

@pinheadmz
Copy link
Member

Thanks for the test Matt. As a quick hunch I tested against other wallet bug fix branches #333 and #262

This was not successful but still might be related. I think we need to check the paths for adding bids to the wallet when rescanning.

@pinheadmz
Copy link
Member

I think I've tracked down the bug. In bcoin/hsd we have two balances unconfirmed and confirmed. People are often confused about this because unconfirmed is actually the TOTAL amount of coins that will be confirmed AFTER everything in the mempool is mined. (most wallets use the term "unconfirmed" only to represent the value in the mempool).

This convention is currently broken in the "locked" values in a wallet balance. Meaning, the lockedUnconfirmed currently only represents unconfirmed coins, not what the total will be after everything gets mined. This is never a problem during regular wallet use because every TX whether its a send, bid, open, reveal, etc will always be inserted into the txdb as an UNCONFIRMED TX when it is broadcast. During a rescan, this is not the case -- the TX is inserted as CONFIRMED without ever having first been UNCONFIRMED.

I have written a patch that passes @mslipper's test and will post shortly.

@buffrr
Copy link
Contributor Author

buffrr commented Mar 9, 2020

@pinheadmz I tried the patch you posted and it does seem to fix this issue.
I tested this on mainnet (with a new hsd node that has the patch). Did the same import process and I'm able to use the wallet & send updates without issues.

@pinheadmz
Copy link
Member

@buffrr thanks but don't run this on mainnet. I already found a big problem with my patch (take a look at your balance -- is the lockedUnconfirmed twice what you expect?)

@pinheadmz
Copy link
Member

OK, this patch is more correct I think: #387

And includes tests which run through both a "regular" auction and a rescanned auction.

@buffrr
Copy link
Contributor Author

buffrr commented Mar 11, 2020

yeah no problem i just saw the PR and couldn't resist :) I didn't keep the node so I'm not sure what the value of lockedUnconfirmed was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants