[script] signet's getcoins.py improvements #22565

pull NikhilBartwal wants to merge 2 commits into bitcoin:master from NikhilBartwal:update_signet_getcoins changing 1 files +31 −5
  1. NikhilBartwal commented at 8:24 PM on July 27, 2021: contributor

    Currently, using the getcoins.py with a custom signet executes successfully and shows the transfer of 0.001 testBTC as complete, however for obvious reasons, it should not. In fact, upon verification it does not actually execute the transaction, but rather gives the output that it did, as shown below which can be misleading:

    [nikhilb@nikhil-PC bitcoin]$ echo $datadir
    /home/nikhilb/signet-custom
    [nikhilb@nikhil-PC bitcoin]$ contrib/signet/getcoins.py -- -datadir=$datadir
    Payment of 0.00100000 BTC sent with txid dd22c7d996e95f3e5baf20f73140d517ff48f1b26d0e4fefd61e3c37991b8f86
    [nikhilb@nikhil-PC bitcoin]$ bitcoin-cli -datadir=$datadir getrawtransaction dd22c7d996e95f3e5baf20f73140d517ff48f1b26d0e4fefd61e3c37991b8f86
    error code: -5
    error message:
    No such mempool or blockchain transaction. Use gettransaction for wallet transactions.
    [nikhilb@nikhil-PC bitcoin]$ bitcoin-cli -datadir=$datadir gettransaction dd22c7d996e95f3e5baf20f73140d517ff48f1b26d0e4fefd61e3c37991b8f86
    error code: -5
    error message:
    Invalid or non-wallet transaction id
    
    

    This PR adds a sanity check for custom signet by comparing the current network's first block hash (the block after the genesis block) with global signet's respective block hash (since all signet networks share the same genesis block) and if a custom network is detected, the user is prompted to either work on the global signet or setup their own faucet.

    The PR was checked to be working successfully, giving the output as below:

    [nikhilb@nikhil-PC bitcoin]$ git checkout update_signet_getcoins
    Switched to branch 'update_signet_getcoins'
    Your branch is ahead of 'upstream/master' by 1 commit.
      (use "git push" to publish your local commits)
    [nikhilb@nikhil-PC bitcoin]$ contrib/signet/getcoins.py -- -datadir=$datadir
    The global faucet cannot be used with a custom Signet network. Please use the global signet or setup your custom faucet for the same.
    
    You can have a look here for setting up your own faucet: https://en.bitcoin.it/wiki/Signet
    
    
  2. NikhilBartwal marked this as a draft on Jul 27, 2021
  3. NikhilBartwal commented at 8:41 PM on July 27, 2021: contributor

    In addition to the above, since the faucet limit is once per 24 hrs for each IP address (for the script), getcoins.py can only be used once for each IP. However, using it twice gives a vague output like this:

    [nikhilb@nikhil-PC bitcoin]$ contrib/signet/getcoins.py 
    Captcha required (reload page)
    
    

    It would be better if the output provided is something on the lines of:

    Daily limit exhausted for signet faucet. Please try again after 24hrs
    

    Will be pushing a commit soon for the doc update as well as a cleaner error :)

  4. NikhilBartwal renamed this:
    [script] Update signet's getcoins.py for handling custom signet network
    [script] Signet's getcoins.py improvements
    on Jul 27, 2021
  5. DrahtBot added the label Scripts and tools on Jul 27, 2021
  6. NikhilBartwal commented at 8:18 PM on July 28, 2021: contributor

    Currently, there are some scenarios where the faucet transaction can fail, some of which are:

    • If the script is run with the same IP within 24hrs (Status code 400)
    • If the specified faucet URL is invalid (Status code 404)
    • If the provided signet address is invalid (Status code 504)

    Originally, the script outputs a vague Captcha required (reload page) error for the first scenario and some weird raw HTML for the other two, which seems confusing. It is, therefore, better to have explicit errors for them.

  7. NikhilBartwal renamed this:
    [script] Signet's getcoins.py improvements
    [script] signet's getcoins.py improvements
    on Jul 28, 2021
  8. NikhilBartwal marked this as ready for review on Jul 28, 2021
  9. NikhilBartwal force-pushed on Jul 29, 2021
  10. NikhilBartwal force-pushed on Jul 29, 2021
  11. NikhilBartwal force-pushed on Jul 29, 2021
  12. in contrib/signet/getcoins.py:24 in dda15ee260 outdated
      21 |  parser.add_argument('bitcoin_cli_args', nargs='*', help='Arguments to pass on to bitcoin-cli (default: -signet)')
      22 |  
      23 |  args = parser.parse_args()
      24 |  
      25 | +if args.faucet.lower() == DEFAULT_GLOBAL_FAUCET:
      26 | +    # Get the first block hash for the current signet network
    


    0xB10C commented at 10:35 AM on July 29, 2021:

    nit: I find the following comment clearer:

    # Get the hash of the block at height 1 of the currently active signet chain
    

    NikhilBartwal commented at 6:48 AM on July 30, 2021:

    Sure, block height makes sense :)

  13. in contrib/signet/getcoins.py:31 in dda15ee260 outdated
      28 | +        curr_signet_hash = subprocess.check_output([args.cmd] + args.bitcoin_cli_args + ['getblockhash', '1']).strip().decode()
      29 | +    except FileNotFoundError:
      30 | +        print('The binary', args.cmd, 'could not be found.')
      31 | +        exit()
      32 | +    if curr_signet_hash != GLOBAL_FIRST_BLOCK_HASH:
      33 | +        # Becomes true for custom signet
    


    0xB10C commented at 10:37 AM on July 29, 2021:

    nit: comment not needed

  14. in contrib/signet/getcoins.py:33 in dda15ee260 outdated
      30 | +        print('The binary', args.cmd, 'could not be found.')
      31 | +        exit()
      32 | +    if curr_signet_hash != GLOBAL_FIRST_BLOCK_HASH:
      33 | +        # Becomes true for custom signet
      34 | +        print('The global faucet cannot be used with a custom Signet network. Please use the global signet or setup your custom faucet for the same.\n')
      35 | +        print('You can have a look here for setting up your own faucet: https://en.bitcoin.it/wiki/Signet')
    


    0xB10C commented at 10:42 AM on July 29, 2021:

    en.bitcoin.it/wiki/Signet only lists two faucet repositories, but does not provide setup instructions. Might be better to drop this message.


    NikhilBartwal commented at 6:49 AM on July 30, 2021:

    Ah, alright!

  15. in contrib/signet/getcoins.py:34 in 57c9ccc40e outdated
      28 | @@ -29,7 +29,7 @@
      29 |          exit()
      30 |      if curr_signet_hash != GLOBAL_FIRST_BLOCK_HASH:
      31 |          # Becomes true for custom signet
      32 | -        print('The global faucet cannot be used with a custom Signet network. Please use the global signet or setup your custom faucet for the same.\n')
      33 | +        print('The global faucet cannot be used with a custom Signet network. Please use the global signet or setup your custom faucet to use this functionality.\n')
    


    0xB10C commented at 10:42 AM on July 29, 2021:

    should be in the first commit

  16. in contrib/signet/getcoins.py:62 in 57c9ccc40e outdated
      60 | +elif res.status_code == 404:
      61 | +    print('The specified faucet URL does not exist. Please check for any server issues/typo.')
      62 | +elif res.status_code == 504:
      63 | +    print('The specified signet address is not valid. Please check it and try again.')
      64 | +else:
      65 | +    print('Unfortunately, the transaction could not be processed. Please check all arguments and try again.')
    


    0xB10C commented at 10:48 AM on July 29, 2021:

    This catch-all could have a broader error message. It's not always the transaction that can't be processed.

    Maybe The request could not be processed. and then print the server response.


    NikhilBartwal commented at 6:51 AM on July 30, 2021:

    Yes, I agree that having such detailed error handling might be confusing and not the best to have. Maybe we can have an explicit one for 404 and a common one for all others for now.

  17. in contrib/signet/getcoins.py:55 in 57c9ccc40e outdated
      53 | +
      54 | +# Display the output as per the returned status code
      55 | +if res:
      56 | +    # When the return code in between 200 and 400 i.e. successful
      57 | +    print(res.text)
      58 | +elif res.status_code == 400:
    


    0xB10C commented at 11:01 AM on July 29, 2021:

    The faucet should return "429 Too Many Requests" which indicates rate-limiting. Returning "400 Bad Request" is incorrect HTTP. Could be changed here.


    0xB10C commented at 11:06 AM on July 29, 2021:

    The faucet returns "400 Bad Request" in multiple places. So status 400 doesn't have to be rate limiting. Better not match on it.

  18. in contrib/signet/getcoins.py:60 in 57c9ccc40e outdated
      58 | +elif res.status_code == 400:
      59 | +    print('You can use the global faucet only once per 24hrs. Please try again later.')
      60 | +elif res.status_code == 404:
      61 | +    print('The specified faucet URL does not exist. Please check for any server issues/typo.')
      62 | +elif res.status_code == 504:
      63 | +    print('The specified signet address is not valid. Please check it and try again.')
    


    0xB10C commented at 11:06 AM on July 29, 2021:

    Not sure about printing address not valid for a status code that should be 504 Gateway Timeout. There might be different reasons for a 504.


    NikhilBartwal commented at 6:52 AM on July 30, 2021:

    Makes sense :)


    NikhilBartwal commented at 6:57 AM on July 30, 2021:

    Actually, when 504 happens, it returns a huge chunk of raw HTML which might look weird if we print it as output.

  19. 0xB10C changes_requested
  20. 0xB10C commented at 11:17 AM on July 29, 2021: member

    Concept ACK on adding a chain sanity check and for improving the error reporting. I agree that the current error reporting is confusing.

    However, I do think the error reporting doesn't have to be this detailed as it might introduce new confusing behavior (see some of my comments below). Just printing the status code, the server message, and a notice about eventual rate-limiting could be better. This also requires less maintenance if the faucets code changes.

  21. ghost commented at 11:57 AM on July 29, 2021: none

    Agree with most of the comments by 0xb10c. Adding few more things which may help in improving this script:

    In addition to the above, since the faucet limit is once per 24 hrs for each IP address, getcoins.py can only be used once for each IP. However, using it twice gives a vague output like this: Captcha required (reload page)

    It would be better if the output provided is something on the lines of: Daily limit exhausted for signet faucet. Please try again after 24hrs

    I think there is no limit and users don't need to wait for 24 hours. They just need to enter captcha or use new IP. As a user or dev I would solve this problem with one of the below solutions:

    1. Use new IP for each request. Maybe new tor circuit while using torsocks.
    2. Show captcha image with python code.
    3. Not use script. Open website in browser, enter captcha and get some coins.
  22. 0xB10C commented at 12:35 PM on July 29, 2021: member

    I think there is no limit and users don't need to wait for 24 hours. They just need to enter captcha or use new IP. As a user or dev I would solve this problem with one of the below solutions:

    • Use new IP for each request. Maybe new tor circuit while using torsocks.
    • Show captcha image with python code.
    • Not use script. Open website in browser with new IP and get some coins.

    Kalle Alm has implemented the rate-limiting for a reason. There is no need to game these limits or to make the script more complex than it has to be (with e.g. tor or rendering the captcha SVGs via python)...

  23. ghost commented at 12:48 PM on July 29, 2021: none

    There is no need to game these limits or to make the script more complex than it has to be (with e.g. tor or rendering the captcha SVGs via python)

    Then maybe 3 can be used. And no need to mislead users that they should wait for 24 hours.

  24. in contrib/signet/getcoins.py:56 in 57c9ccc40e outdated
      54 | +# Display the output as per the returned status code
      55 | +if res:
      56 | +    # When the return code in between 200 and 400 i.e. successful
      57 | +    print(res.text)
      58 | +elif res.status_code == 400:
      59 | +    print('You can use the global faucet only once per 24hrs. Please try again later.')
    


    unknown commented at 1:27 PM on July 29, 2021:
    webbrowser.open('https://signetfaucet.com/', new = 1)
    

    0xB10C commented at 11:19 AM on July 30, 2021:

    getcoins.py is a script for a reason. You can't assume that every machine has a GUI or even a browser installed. Especially if this script is used in some automated way.

  25. NikhilBartwal closed this on Jul 30, 2021

  26. NikhilBartwal force-pushed on Jul 30, 2021
  27. NikhilBartwal reopened this on Jul 30, 2021

  28. NikhilBartwal force-pushed on Jul 30, 2021
  29. NikhilBartwal force-pushed on Jul 30, 2021
  30. NikhilBartwal force-pushed on Jul 30, 2021
  31. NikhilBartwal force-pushed on Jul 30, 2021
  32. NikhilBartwal force-pushed on Jul 30, 2021
  33. in contrib/signet/getcoins.py:60 in abba5de330 outdated
      56 | +elif res.status_code == 404:
      57 | +    print('The specified faucet URL does not exist. Please check for any server issues/typo.')
      58 | +else:
      59 | +    print(f'Returned Error Code {res.status_code}\n{res.text}\n')
      60 | +    print('Please check the provided arguments for their validity/any possible typo.')
      61 | +    print('NOTE: The global faucet is currently rate-limited to 1 request per IP per 24hrs. If that\'s the case, please try again after the time limit!')
    


    unknown commented at 10:14 AM on July 30, 2021:

    There is no such thing. Error is misleading.

    Used the same IP for these 2 transactions: https://explorer.bc-2.jp/tx/e18da1b1df25376ace8b75fcab9c54c72a601cb52c04568da2b52dfc61ebec18 https://explorer.bc-2.jp/tx/17770112c1939212bcc20903a686335296a7713eb80654ac6ef411a96c747bbd

    Just had to open https://signetfaucet.com/ in browser and enter details.

    If the error isn't changed, it's a NACK from me.


    NikhilBartwal commented at 11:12 AM on July 30, 2021:

    Actually, the faucet only asks for the captcha for the second (or more) time usage from the same IP, not the first time. Hence, the script in its current form cannot be used more than once in a while (as the faucet needs some hours before being used without captcha from the same IP).

    That's why it kindof acts like a rate limiter (for the current script). The actual rate limits are 1 btc per hr or 500 requests per week.

    So now, we can either state explicitly that the script currently supports single time usage only and they can use the browser manually for more or we would have to upgrade the script for captcha (which IMO would not be worth the maintenance). @0xB10C Would love to hear your thoughts


    0xB10C commented at 11:17 AM on July 30, 2021:

    Just had to open https://signetfaucet.com/ in browser and enter details.

    This is missing the point. This changes the getcoins.py script. You can't assume that because something works in the browser, it works in the script. The script might be used automatically where you can't open a browser.

    Furthermore, this is in an else branch and only printed IF an error occurs. Additionally, it clearly says NOTE informing the user that this might be the reason for failure.

    Based on #22565 (comment) I think @NikhilBartwal ran into this problem when using the script and I think it's reasonable to mention it if an error occurs.

  34. theStack commented at 8:26 PM on July 31, 2021: member

    Concept ACK

  35. 0xB10C commented at 10:08 AM on August 3, 2021: member

    cc @kallewoof you might be interested in reviewing this as it touches your signet faucet.

    Do you have the time to work on the faucet (or review a PR to the faucet) at the moment? See e.g. #22565 (review).

  36. kallewoof commented at 10:24 AM on August 3, 2021: member

    Will review. Some general comments:

    • I am willing to change the faucet if it's warranted.
    • I do think 1 no-captcha request/day/IP is reasonable. The getcoins script gets you enough coins to get started, and I don't see a point in needing to do multiple requests each day, especially if you are automating the process.
    • If you're not automating the process, open a browser and answer the captcha. Assumably once you have coins, you won't have to re-do this.
    • I do think the getcoins script should inform the user of this fact, i.e. "You need to open in a browser and solve the captcha to get multiple pay-outs in a single day."
    • And please don't use multiple IP's etc to get around the faucet rate limiting...
  37. NikhilBartwal commented at 12:52 PM on August 3, 2021: contributor

    @kallewoof I agree with the points mentioned and the rate limiting. The PR, in its current form gives a somewhat better error message in case the Faucet Transaction fails.

    However, I think it might be even better if we could update the faucet to return a Code '429 Too many requests' error when accessing the faucet more than once with the script, as suggested by @0xB10C here so that we can provide a more cleaner message to use the browser for multiple pay-outs.

    Let me know your views on it :)

  38. kallewoof commented at 12:08 AM on August 4, 2021: member

    I've changed to return code 429 in two places:

    • when user tries to get coins a second time without providing a captcha answer
    • when user tries to get coins so much that they hit the daily limit (even with captcha's)
  39. NikhilBartwal force-pushed on Aug 4, 2021
  40. NikhilBartwal commented at 11:02 AM on August 4, 2021: contributor

    Thanks for the update @kallewoof. I've added an explicit error message for multiple-payouts from script and a common one for other possible scenarios. Let me know if it needs any updates @0xB10C.

  41. [script] Update signet getcoins.py for custom network
    Currently, using the getcoins.py with a custom signet executes successfully and shows the transaction as complete, however for obvious reasons, it  should not.
    This PR adds a sanity check for custom signet by comparing the current network's first block hash with global signet's respective hash.
    1c612b274b
  42. Add cleaner errors for unsuccessful faucet transactions b0c8246cac
  43. in contrib/signet/getcoins.py:36 in 495007b144 outdated
      33 | +        print('The global faucet cannot be used with a custom Signet network. Please use the global signet or setup your custom faucet to use this functionality.\n')
      34 | +        exit()
      35 | +
      36 |  if args.addr == '':
      37 |      if args.bitcoin_cli_args == []:
      38 |          args.bitcoin_cli_args = ['-signet']
    


    0xB10C commented at 4:29 PM on August 4, 2021:

    During the getblockhash 1 RPC call -signet isn't set in args.bitcoin_cli_args yet. bitcoin-cli tries to reach the mainnet RPC port 8332.

    You need to set the -signet argument before you do the first RPC call. Probably just move the following up:

     if args.bitcoin_cli_args == []:
            args.bitcoin_cli_args = ['-signet']
    
        

    (I guess this works fine if one sets signet=1 in the bitcoin.conf file, but that can't be assumed to be the case)


    NikhilBartwal commented at 9:31 PM on August 4, 2021:

    Absolutely! I guess I missed this edge case, thanks for mentioning! I've included this in the first commit and pushed the changes :)

  44. NikhilBartwal force-pushed on Aug 4, 2021
  45. in contrib/signet/getcoins.py:59 in b0c8246cac
      55 | +    # When the return code is in between 200 and 400 i.e. successful
      56 | +    print(res.text)
      57 | +elif res.status_code == 404:
      58 | +    print('The specified faucet URL does not exist. Please check for any server issues/typo.')
      59 | +elif res.status_code == 429:
      60 | +    print('The script does not allow for repeated transactions as the global faucet is rate-limitied to 1 request/IP/day. You can access the faucet website to get more coins manually')
    


    unknown commented at 9:53 PM on August 4, 2021:

    This looks better. Thanks for adding that last sentence.

  46. in contrib/signet/getcoins.py:32 in b0c8246cac
      29 | +    # Get the hash of the block at height 1 of the currently active signet chain
      30 | +    try:
      31 | +        curr_signet_hash = subprocess.check_output([args.cmd] + args.bitcoin_cli_args + ['getblockhash', '1']).strip().decode()
      32 | +    except FileNotFoundError:
      33 | +        print('The binary', args.cmd, 'could not be found.')
      34 | +        exit()
    


    0xB10C commented at 8:10 PM on August 5, 2021:

    nit: When using a SigNet with a chain length of 1 (chain = only the Genesis block), this will fail with a Block height out of range error message. This might be confusing for someone unfamiliar with the getcoins.py code. If you happen to touch this again, you could add catch-all except handling here mentioning that checking the hash of the first block to determine the SigNet chain failed. This should also print the RPC response with Block height out of range.

  47. 0xB10C approved
  48. 0xB10C commented at 8:32 PM on August 5, 2021: member

    Tested ACK b0c8246cac97b7792a50afc22ef1fe39c5028e00

  49. theStack approved
  50. theStack commented at 5:42 PM on August 6, 2021: member

    Tested ACK b0c8246cac97b7792a50afc22ef1fe39c5028e00

    Checked that

    • newly introduced error message for code 404 appears by passing -f https://signetfaucet.com/xxxinvalidxxx
    • newly introduced error message for code 429 appears by requesting coins via the global faucet twice in a row (thanks to kallewoof for updating the faucet behaviour in this regard!)
    • newly introduced error message for other error codes appears by passing a random website as faucet, e.g. -f https://orf.at leads to error 403
    • requesting coins with the global faucet in a custom signet leads to the introduced error message
  51. kallewoof commented at 7:07 AM on August 17, 2021: member

    ACK b0c8246cac97b7792a50afc22ef1fe39c5028e00

  52. arnabsen1729 commented at 2:10 PM on August 18, 2021: contributor

    utACK b0c8246

  53. prakash1512 commented at 2:25 PM on August 18, 2021: contributor

    utACK b0c8246

  54. Zero-1729 commented at 10:30 AM on August 21, 2021: contributor

    crACK b0c8246 🧉

  55. laanwj merged this on Aug 23, 2021
  56. laanwj closed this on Aug 23, 2021

  57. MarcoFalke referenced this in commit 718d9f2f77 on Aug 26, 2021
  58. DrahtBot locked this on Aug 23, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 18:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me