bitcoinops / taproot-workshop

Taproot & Schnorr Python Library & Documentation.
MIT License
369 stars 111 forks source link

Sync Colab branch with master #173

Closed mmilata closed 2 years ago

mmilata commented 2 years ago

Issue #172. This is a result of git merge master and resolving the conflict in README.md. Only checked 1.1-schnorr-signatures.ipynb but it seems to work fine.

jnewbery commented 2 years ago

Thanks @mmilata! Are you able to rebase the changes on top of master rather than using git merge?

mmilata commented 2 years ago

Rebased the branch onto master.

0xB10C commented 2 years ago

As mentioned in https://github.com/bitcoinops/taproot-workshop/issues/172#issuecomment-954678700, I think we can and need to drop the custom binary and use a Bitcoin Core release build (either v0.21 or v22.0). The custom binary download is happening in the shell script added by add: script to setup the Google Colab environment. Replacing this with a download and extraction from e.g. https://bitcoincore.org/bin/bitcoin-core-22.0/bitcoin-22.0-x86_64-linux-gnu.tar.gz should do the trick.

Let me know if you need help!

mmilata commented 2 years ago

That makes sense, however with 22.0 (28ebf9dcb88218dfae591b7b0e6a54442219391d) I'm getting (ex 2.0.2 as well as 2.0.6):

JSONRPCException: No wallet is loaded. Load a wallet using loadwallet or create a new one with createwallet. (Note: A default wallet is no longer automatically created)

Same with 0.21. Does 0.21/22.0 work outside google colab?

0xB10C commented 2 years ago

Successfully tested both 2.0.2 and 2.0.6 on a local Jupyter notebook with a self-compiled bitcoind.

The issue with the Colab setup seems to be that this line is commented out.

https://github.com/bitcoinops/taproot-workshop/blob/7e0e628a0e12bbcf39bb624118544b5d796409d8/setup-colab-env.sh#L34

The test framework does not create the default wallet (see https://github.com/bitcoinops/taproot-workshop/pull/168/commits/8e054de3aa19f0f31f5ca46dd27fc84f1bceefa1#diff-bcbd83e29988ebae39a140201447e86bc719b60278bc44dab4f23efa13af7580R100-R102) when it thinks the wallet is not compiled in (set in test/config.ini when running ./configure). I'm not sure why (or if?) this worked in the past. With no # before ENABLE_WALLET=true this successfully runs in Colab.

mmilata commented 2 years ago

Yup, that seems to work. Thanks @0xB10C!

0xB10C commented 2 years ago

Successfully tested this PR on Colab by running this notebook https://colab.research.google.com/github/mmilata/taproot-workshop/blob/colab-update/2.0-taproot-introduction.ipynb and manually changing the the setup-colab-env.sh script URL:

- !curl -o- -s -S -L https://raw.githubusercontent.com/bitcoinops/taproot-workshop/Colab/setup-colab-env.sh | bash
+ !curl -o- -s -S -L https://raw.githubusercontent.com/mmilata/taproot-workshop/colab-update/setup-colab-env.sh | bash

Not sure why GitHub is complaining This branch has conflicts that must be resolved.

mmilata commented 2 years ago

It's because @jnewbery asked me to rebase the Colab changes on top of master but GitHub thinks we're gonna merge it.

mmilata commented 2 years ago

Ping @jnewbery, should be working now, wanna force-push it? Can't wait to spam my colleagues with the workshop :smiley:

jnewbery commented 2 years ago

Ping @jnewbery, should be working now, wanna force-push it? Can't wait to spam my colleagues with the workshop

Done! Thanks so much @mmilata! (and @0xB10C for testing)