stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 673 forks source link

[TESTNET BUG] BTC faucet can not get result from bitcoin-rpc-api listenunspent #2200

Closed gzelda closed 3 years ago

gzelda commented 3 years ago

Describe the bug When you get btc faucet from following command(assuming address is mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs):

curl -X POST https://stacks-node-api.krypton.blockstack.org/extended/v1/faucets/btc?address=mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs

You will get the faucet in around 5 minutes by:

curl https://stacks-node-api.krypton.blockstack.org/extended/v1/faucets/btc/mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs

But when you start the miner node using this address, you will get UTXOs not found, node will turn to follower node.

stacks-node use listenunspent to judge if address has UTXOs that means balance. But I have tested by postman, listenunspent returns an empty array. 截屏2020-12-21 上午8 50 34

With the suggestion from @diwakergupta, I tried scantxoutset, the result shows there is a record of faucet (which shows the a txid related btc amount is 0.5). 截屏2020-12-21 上午8 50 50

So I think the issue caused by the listenunspent method do not sync with latest burnchain state, if we should use scantxoutset to do UTXO check in stacks-node.

Source code located:

UTXOs check function in stacks-blockchain in neon.rs

It call the function in bitcoin controller

Steps To Reproduce

  1. Use npx @stacks/cli make_keychain -t to create a keychain

  2. Use

    curl -X POST https://stacks-node-api.krypton.blockstack.org/extended/v1/faucets/btc?address=<YOUR_ADDRESS>

    to get the faucet in Krypton

  3. Copy your private key in .toml file. Use stacks-node start --config=<YOUR TOML LOCATION>

  4. You will get UTXOs not found.......

Expected behavior UTXOs found.......

Environment (please complete the following information):

gzelda commented 3 years ago

@CharlieC3

diwakergupta commented 3 years ago

With the suggestion from @diwakergupta, I tried scantxoutset, the result shows there is a record of faucet (which shows the a txid related btc amount is 0.5).

h/t @zone117x

So I think the issue caused by the listenunspent method do not sync with latest burnchain state, if we should use scantxoutset to do UTXO check in stacks-node.

I'd like @lgalabru to weigh in. Matt's hypothesis was that listunspent "will exclude uxtos that are unsuitable for spending for a myriad of reasons, while scantxoutset doesn't"

@tyGavinZJU can you confirm whether this is reliably happening with every address or just some of them?

gzelda commented 3 years ago

@tyGavinZJU can you confirm whether this is reliably happening with every address or just some of them?

From my test, only one address the result of listenunspent sync with the result of scantxoutset. Other ten address all failed.

lgalabru commented 3 years ago

@tyGavinZJU Are you using the master? If that's the case, then I think that the experienced behavior is what was expected. When we released Xenon, we assumed that the miners would start using their own bitcoind instance, and we removed the importaddress step, that was being performed before listing the UTXOs (this step is required if you want to to use the listunspent call).~~ This step was fine on Bitcoin Regtest, but it's an expensive operation on Bitcoin Testnet, that could DDOS our node.

EDIT: I see that this should actually still work, the importaddress is behind a test:

                if let BitcoinNetworkType::Regtest = network {
                    // Performing this operation on Mainnet / Testnet is very expensive, and can be longer than bitcoin block time.
                    // Assuming that miners are in charge of correctly operating their bitcoind nodes sounds
                    // reasonable to me.
                    // $ bitcoin-cli importaddress mxVFsFW5N4mu1HPkxPttorvocvzeZ7KZyk
                    let _result = BitcoinRPCRequest::import_public_key(&self.config, &public_key);
                    sleep_ms(1000);
                }

Do you mind sharing your config?

lgalabru commented 3 years ago

I just tried performing a listunspent operation, and I'm getting the following result:

curl -X "POST" "http://bitcoind.krypton.blockstack.org:18443" \
     -H 'Content-Type: application/json; charset=utf-8' \
     -d $'{
  "id": "stacks",
  "jsonrpc": "2.0",
  "method": "listunspent",
  "params": [
    0,
    9999999,
    [
      "mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs"
    ],
    false,
    {
      "minimumAmount": "0.00000001"
    }
  ]
}'

HTTP/1.1 200 OK
content-length: 634
date: Mon, 21 Dec 2020 03:33:09 GMT
content-type: application/json

{"result":[{"txid":"326c569453abac8b2541e1e82ef5abf134419d933e49285d56d01a86aa2af521","vout":0,"address":"mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs","label":"","scriptPubKey":"76a91426bca0d27a90b5c82f798d7bbb6829737c9adcb988ac","amount":0.50000000,"confirmations":62,"spendable":false,"solvable":false,"safe":true},{"txid":"74aa7ba3e819d6bde4084c194d3479ac59a74a2e86b047c2b0f309cd49708423","vout":0,"address":"mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs","label":"","scriptPubKey":"76a91426bca0d27a90b5c82f798d7bbb6829737c9adcb988ac","amount":0.50000000,"confirmations":75,"spendable":false,"solvable":false,"safe":true}],"error":null,"id":"stacks"}
diwakergupta commented 3 years ago

When we released Xenon, we assumed that the miners would start using their own bitcoind instance, and we removed the importaddress step

That would certainly explain the behavior, thanks @lgalabru

The Krypton miner, and the mining-bot both are currently using v24.0.0.0-xenon, which I believe is indeed missing this step. Here's the relevant diff: https://github.com/blockstack/stacks-blockchain/compare/v23.0.0.12-krypton...v24.0.0.0-xenon#diff-06a6e88675e542d59fefd0f0e8b3738028510b196074d2a7c77a07f79383a79cL462

I think the simplest solution might be for @tyGavinZJU to update the mining-bot to use v23.0.0.12-krypton? https://github.com/blockstack/stacks-blockchain/releases/tag/v23.0.0.12-krypton

If there were consensus breaking changes between that and v24.0.0.0-xenon, than we would additionally have to downgrade the Krypton nodes to use the v23 build.

Thoughts @lgalabru ?

diwakergupta commented 3 years ago

Another (simpler) alternative might be for the mining-bot to issue the importaddress command before starting the node? Wouldn't require any changes to the Krypton setup and no change to the build used by the mining-bot. WDYT @tyGavinZJU

Though I'm not sure how bitcoind will behave if it's tracking UTXOs for 100s of addresses -- don't think we've ever run with that kind of load (just FYI @CharlieC3)

gzelda commented 3 years ago

Another (simpler) alternative might be for the mining-bot to issue the importaddress command before starting the node? Wouldn't require any changes to the Krypton setup and no change to the build used by the mining-bot. WDYT @tyGavinZJU

Any rpc api for importaddress?

gzelda commented 3 years ago

I just tried performing a listunspent operation, and I'm getting the following result:

curl -X "POST" "http://bitcoind.krypton.blockstack.org:18443" \
     -H 'Content-Type: application/json; charset=utf-8' \
     -d $'{
  "id": "stacks",
  "jsonrpc": "2.0",
  "method": "listunspent",
  "params": [
    0,
    9999999,
    [
      "mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs"
    ],
    false,
    {
      "minimumAmount": "0.00000001"
    }
  ]
}'

HTTP/1.1 200 OK
content-length: 634
date: Mon, 21 Dec 2020 03:33:09 GMT
content-type: application/json

{"result":[{"txid":"326c569453abac8b2541e1e82ef5abf134419d933e49285d56d01a86aa2af521","vout":0,"address":"mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs","label":"","scriptPubKey":"76a91426bca0d27a90b5c82f798d7bbb6829737c9adcb988ac","amount":0.50000000,"confirmations":62,"spendable":false,"solvable":false,"safe":true},{"txid":"74aa7ba3e819d6bde4084c194d3479ac59a74a2e86b047c2b0f309cd49708423","vout":0,"address":"mj3mwTihYQBvKrhdrGUneT17RoiJ1tMxVs","label":"","scriptPubKey":"76a91426bca0d27a90b5c82f798d7bbb6829737c9adcb988ac","amount":0.50000000,"confirmations":75,"spendable":false,"solvable":false,"safe":true}],"error":null,"id":"stacks"}

The result sometimes succeeds after a really long time.

diwakergupta commented 3 years ago

Another (simpler) alternative might be for the mining-bot to issue the importaddress command before starting the node? Wouldn't require any changes to the Krypton setup and no change to the build used by the mining-bot. WDYT @tyGavinZJU

Any rpc api for importaddress?

https://bitcoincore.org/en/doc/0.18.0/rpc/wallet/importaddress/ ?

Actually, if @tyGavinZJU can supply a list of known BTC addresses for participants in the competition, the simplest solution might be for us to just run importaddress on the Krypton bitcoind directly.

@lgalabru if you can confirm that the importaddress step is indeed missing in v24.0.0.0-xenon and above sounds like a reasonable option, perhaps @CharlieC3 or @wileyj can help execute?

gzelda commented 3 years ago

https://bitcoincore.org/en/doc/0.18.0/rpc/wallet/importaddress/ ?

I want to ask what param should use, I refer to address which is imported in wallet. The label to import should use "testing".

Here is the importaddress operation which worked for me.

curl -X "POST" "http://bitcoind.krypton.blockstack.org:18443" \
     -H 'Content-Type: application/json; charset=utf-8' \
     -d $'{
    "id":"stacks",
    "jsonrpc":"2.0",
    "method":"importaddress",
    "params":["<YOUR_ADDRESS>","testing",false]
}'

Actually, if @tyGavinZJU can supply a list of known BTC addresses for participants in the competition, the simplest solution might be for us to just run importaddress on the Krypton bitcoind directly.

Already integrated in mining-bot. Will do it when miner want to start mining. Will it cause unexpected issue in krypton bitcoin node?

Thanks @diwakergupta @lgalabru

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

blockstack-devops commented 2 weeks ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.