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

Raspberry Pi check; fixing breaking changes when not using built-in Tor #1037

Merged
merged 18 commits into from Mar 22, 2021

Conversation

kdmukai
Copy link
Collaborator

@kdmukai kdmukai commented Mar 21, 2021

Raspberry Pi cannot use the Linux binaries for Tor Browser or bitcoind, unfortunately. So this detects Raspberry Pi OS and throws up error messages:
Screen Shot 2021-03-20 at 10 32 57 PM


Changes for the new built-in Tor support break external Tor setups due to the assumption that there will now be a torrc_password in Specter config.json (instead of using CookieAuthentication in torrc). It wasn't clear to me if there was any way to support both HashedControlPassword and CookieAuthentication.

Adds a check for torrc_password and writes it to config.json if missing.

tor.md docs updated accordingly with manual instructions for hashing the password.

It seemed easiest to add this check in update_tor_controller but perhaps there's a better place for it.

Added generate_torrc_password() for DRY consolidation and seems a bit cleaner to not have tor_setup_tasks.py calling the specter object's _save().

@kdmukai
Copy link
Collaborator Author

kdmukai commented Mar 21, 2021

Raspi is now failing on the assignment to the bitcoind attribute:

        specter.config["bitcoind_setup"]["stage"] = "Starting up Bitcoin Core..."
        specter._save()
        specter.bitcoind = BitcoindPlainController(
            bitcoind_path=specter.bitcoind_path,
            rpcport=8332,
            network="mainnet",
            rpcuser=specter.config["rpc"]["user"],
            rpcpassword=specter.config["rpc"]["password"],
        )

see: https://github.com/cryptoadvance/specter-desktop/blob/master/src/cryptoadvance/specter/util/bitcoind_setup_tasks.py#L168

Traceback (most recent call last):
  File "/home/pi/specter-desktop/src/cryptoadvance/specter/util/bitcoind_setup_tasks.py", line 177, in setup_bitcoind_directory_thread
    rpcpassword=specter.config["rpc"]["password"],
AttributeError: can't set attribute

Looks like a mismatch with the @property definition of Specter.bitcoind that has no @property.setter:
https://github.com/cryptoadvance/specter-desktop/blob/master/src/cryptoadvance/specter/util/bitcoind_setup_tasks.py#L168

I don't get how this worked elsewhere. Running python 3.7.3. Do properties maybe work differently in newer versions?

Currently testing this workaround:

        specter.config["bitcoind_setup"]["stage"] = "Starting up Bitcoin Core..."
        specter._save()

        # Specter's 'bitcoind' attribute will instantiate a BitcoindController as needed
        specter.bitcoind.start_bitcoind(
            datadir=os.path.expanduser(specter.config["rpc"]["datadir"])
        )

@kdmukai
Copy link
Collaborator Author

kdmukai commented Mar 21, 2021

Can now confirm that a new clean run with this code (wiped out ~/.bitcoin and ~/.specter) on the raspi4 is working!

Had to extend the wait_for_bitcoind timeout on the raspi4 since it takes longer than 30s to get bitcoind up and running (see most recent commit above).

Screen Shot 2021-03-21 at 11 09 13 AM

@kdmukai
Copy link
Collaborator Author

kdmukai commented Mar 21, 2021

Confirmed same can't set attribute bug in Windows 10, python 3.8.8 running from source from current master. The executable specterd ran fine on Win10 before. Must have been a regression that crept in since that was built.

Switching to the code in this PR resolves the issue. Win10 Tor + Bitcoin Core up and running*!

  • Ugh, Windows is a mess. I had previously run the specterd executable to test the wizard and it ran fine. To reset the machine I deleted ~/.specter and /Users/Keith/AppData/Roaming/Bitcoin (equivalent to ~/.bitcoin on linux). When running from source the Bitcoin folder wasn't being created in Roaming. The whole AppData dir is set to Read-only for some reason. I manually created the Bitcoin folder in there and the usual bitcoin core files and dirs started showing up once bitcoind was started. But weirdly there was no bitcoin.conf.

I eventually found it in C:\Users\Keith\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\LocalCache\Roaming\Bitcoin

The QuickSync blocks were in there, too.

So it looks like my python execution environment is isolating the file writes. But when bitcoind starts, it's still looking for its data dir in the normal place. So the node IS running, but now it's running with no bitcoin.conf and is therefore behaving like a new full node doing IBD.

I don't know if this is peculiar to my Win10 setup. I just installed python 3.8 via the Microsoft Store earlier today and that's what I'm running against. Looks to me like a permissions/data siloing thing going on.

So was the original specterd executable doing the right thing? Not sure.

@kdmukai
Copy link
Collaborator Author

kdmukai commented Mar 22, 2021

Solved!

The big mistake was installing Python 3.8.8 from the Microsoft Store. That's why it was running in that Local\Packages subdir. Installing from a python.org download and running everything in PowerShell has completely resolved all problems on Windows 10!

Updated the docs accordingly.

@k9ert
Copy link
Collaborator

k9ert commented Mar 22, 2021

Great! I've created a pre-release on this branch and it worked really smooth on windows (apart from the security-issues mentioned above).
Let's do the rest of the testing with a combined pre-release. Merging!

@k9ert k9ert merged commit 6e7a340 into cryptoadvance:master Mar 22, 2021
@kdmukai
Copy link
Collaborator Author

kdmukai commented Mar 22, 2021

Testing on a clean Win10 with the pre-release binaries worked great!

Ancient Macbook Air needs a timeout extension (see #1044) but otherwise also working great from latest source (no DMG to play with in the pre-release yet).

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

Successfully merging this pull request may close these issues.

None yet

3 participants