X-CASH-official / xcash-dpops

🗳 Delegated-Proof-of-Private-Stake: First DPoS implementation on a Monero-based coin
https://xcash.foundation
MIT License
48 stars 17 forks source link

[bug] Installation fails creating a new wallet as sudo user #42

Closed xcash-rocks closed 4 years ago

xcash-rocks commented 4 years ago

🐛 BUG REPORT

Installation fails creating a new wallet as sudo user See Screenshot: https://i.imgur.com/X8oR4Z1.png

Expected Behavior

Should return X-Cash Delegate wallet data information and create the wallet and keys.

Current Behavior

No X-Cash Delegate wallet data information promted and no wallet been created

Steps to Reproduce (for bugs)

  1. bash -c "$(curl -sSL https://raw.githubusercontent.com/X-CASH-official/xcash-dpops/master/scripts/autoinstaller/autoinstaller.sh)"
  2. Enter the number of the installation type: 1
  3. ls -la xcash-official/xcash-wallets/

Your Environment

Ubuntu 18.04.5 LTS

n3me5is-git commented 4 years ago

Probably this is solved inside the PR: https://github.com/X-CASH-official/xcash-dpops/pull/41 In particular with this commit: https://github.com/X-CASH-official/xcash-dpops/pull/41/commits/239f9a921565b59e1c492ce8f207fdb5ecada480

In my case there were problems during the importing phase, but maybe it's the same bug that affects different conditions...

zachhildreth commented 4 years ago

Thanks @n3me5is-git I see you updated you PR with the autoinstaller fixes!

I will be testing these out today and pulling them in tomorrow

xcash-rocks commented 4 years ago

Wallet creation still not working with the new PR: https://i.imgur.com/BeNZ63N.png

Will give another try with more debug echo's

n3me5is-git commented 4 years ago

Mmmm. I will check.... But this time the problem is when importing wallet, not before.... So the wallet creation works. Before the Wallet import the script removes the wallet files and re-create them. I think this is done because in the past there was an issue with "0 balance" in some cases, so the script first creates the new wallet and then reimport it

zachhildreth commented 4 years ago

Before the Wallet import the script removes the wallet files and re-create them. I think this is done because in the past there was an issue with "0 balance" in some cases, so the script first creates the new wallet and then reimport it

Yes this is why I added it on there

n3me5is-git commented 4 years ago

Maybe found the issue. A missing variable declaration Try add this variable initialization to the script, for example after WALLET_SEED=""

MNEMONIC_SEED=""

The script uses 2 different names, one when importing the wallet "manually" and the other when re-creating the wallet, using the data extracted in get_current_xcash_wallet_data().

n3me5is-git commented 4 years ago

Can you try if it works? Because the strange thing is that it worked in my installation, the 2 times I tested it. I recorded also a video! Very strange...

This is the code:

    sudo systemctl start xcash-daemon &>/dev/null
    sleep 30s

    (echo -ne "\n"; echo "${WALLET_PASSWORD}"; echo "exit") | "${XCASH_DIR}"build/release/bin/xcash-wallet-cli --restore-deterministic-wallet --electrum-seed "${MNEMONIC_SEED}" --generate-new-wallet "${XCASH_DPOPS_INSTALLATION_DIR}"xcash-wallets/delegate-wallet --password "${WALLET_PASSWORD}" --mnemonic-language English --restore-height 0 --trusted-daemon &>/dev/null

    sudo systemctl stop xcash-daemon &>/dev/null
    sleep 30s

So it can crash only during wallet restoration... The start/stop daemon part is the same if the create_wallet and sync_wallet functions.

n3me5is-git commented 4 years ago

Added the missing variable to the PR. I hope this is the cause of the issue, otherwise we need to investigate further

As you can see here, my installation in working. And used the same script of the PR. Strange. https://user-images.githubusercontent.com/68710094/93710864-0c419e00-fb4a-11ea-90d2-e45f57f9c3f7.gif

xcash-rocks commented 4 years ago

testing your fix...
slow vps needs its time.

zachhildreth commented 4 years ago

it still crashes at that part for me

zachhildreth commented 4 years ago

@n3me5is-git

can you make these changes to your PR #41 In the 3 places that it has Importing X-CASH Wallet (This Might Take A While)

1 can you add cd "${XCASH_DPOPS_INSTALLATION_DIR}" in front of rm -f "${XCASH_DPOPS_INSTALLATION_DIR}"xcash-wallets/delegate-wallet*

2 can you add sudo systemctl stop xcash-daemon in front of sudo systemctl start xcash-daemon

doing this seems to fix it, for both create and import wallet options for the "Install" menu selection

also uninstall seems to work! however can you remove these files as well and add it to your PR #41

/lib/systemd/system/xcash-dpops.timer
/lib/systemd/system/xcash-rpc-wallet.timer
${HOME}/firewall_script.sh

Thanks!

n3me5is-git commented 4 years ago

@zachhildreth Added the modifications. Introduced also a small 5s sleep between the sudo systemctl stop xcash-daemon and the sudo systemctl start xcash-daemon. Please check: https://github.com/n3me5is-git/xcash-dpops/commit/3f4464e9e97ccc5f1996e7aa20a7890d380aa263

The strange thing is that I never had problems, so maybe is the bug related to some "race conditions" when starting/stopping the services? I can not explain why on my 2 VM everything worked fine... Especially during the video recording (maybe the script has a personality XD).

However, I hope the major issues are solved now!

You can do the last check and merge!

n3me5is-git commented 4 years ago

P.S my PR include also the other 2 PR now, so you can apply the other 2 before and then the big PR

xcash-rocks commented 4 years ago

FYI: currently, running the installation with the fixes above proposed by zach and the installer freezes while "Syncing X-CASH Wallet".

Testing now the fresh PR with commit 3f4464e & will report asap.

n3me5is-git commented 4 years ago

@xcash-rocks The installer crashes or only freezes? Because during one of my installations this step took about 15min to execute... If you use byobu (or screen) you can open a new terminal and check these things while the installation is stuck

I will add sudo systemctl stop xcash-daemon in front of sudo systemctl start xcash-daemon also in the sync part to be sure. Just in case...

xcash-rocks commented 4 years ago

@n3me5is-git

@xcash-rocks The installer crashes or only freezes? Because during one of my installations this step took about 15min to execute...

frozen ... took more than 40minutes, till I killed the script and no wallet was created

If you use byobu (or screen) you can open a new terminal and check these things while the installation is stuck

  • If there are the wallet files in the wallet folder
  • Use htop to see the active processes
  • Use systemctl and journalctl check if the daemon is alive and in case the errors In case of a new install, the daemon must synchronize the missing blocks, and then do the rescan

Will do those checks with the current running test-installation!

xcash-rocks commented 4 years ago

@n3me5is-git
Still no X-Cash Delegate wallet data information posted and no wallet been created: See: https://i.imgur.com/sk5KWBy.png

Will retry now with commit e9d7c74 and some debug echo's

n3me5is-git commented 4 years ago

@xcash-rocks So this time the "Syncing X-CASH Wallet" worked and the issue returned with "Importing X-CASH"... There is something strange when using the local daemon to sync/importing the wallet but this is really strange... Maybe you can try to execute manually

Try to replace in the script this

(echo -ne "\n"; echo YOUR_WALLET_PASSWORD; echo "exit") | "${XCASH_DIR}"build/release/bin/xcash-wallet-cli --restore-deterministic-wallet --electrum-seed "${MNEMONIC_SEED}" --generate-new-wallet "${XCASH_DPOPS_INSTALLATION_DIR}"xcash-wallets/delegate-wallet --password "${WALLET_PASSWORD}" --mnemonic-language English --restore-height 0 --trusted-daemon &>/dev/null

with this

(echo -ne "\n"; echo YOUR_WALLET_PASSWORD; echo "exit") | "${XCASH_DIR}"build/release/bin/xcash-wallet-cli --restore-deterministic-wallet --electrum-seed "${MNEMONIC_SEED}" --generate-new-wallet "${XCASH_DPOPS_INSTALLATION_DIR}"xcash-wallets/delegate-wallet --password "${WALLET_PASSWORD}" --mnemonic-language English --restore-height 0 --trusted-daemon

Basically removing the &>/dev/null so you can see the output of that command. You will see what the wallet is doing when importing from seed... You can also add a sudo systemctl status xcash-daemon after the .... start .... to debug.

n3me5is-git commented 4 years ago

Maybe you can remove the &>/dev/null also in the functions used to generate the wallet and sync the wallet, so you can debug what the wallet-cli is doing during the whole process. Maybe this can highlight better where is the problem... Because it seems that the problem is sometimes with sync and sometimes with import. But more with import, that executes after the get_current_xcash_wallet_data() using the data extracted from this function. If you want to see the extracted data before the crash you can add an echo -e "${CURRENT_XCASH_WALLET_INFORMATION}" just after that function call (line 1855 more or less), so you can see if the data is extracted correctly. In particular, the seed, password and public address are used for the next operations, so if the seed is not correct the import wallet code will fail.

Basically the import code can fail when:

zachhildreth commented 4 years ago

I tried it again with the new PR and it does not work either. It exits the script at importing

testing some other things

zachhildreth commented 4 years ago

can @n3me5is-git or @xcash-rocks confirm that changing sleep 5 to sleep 30s on the import code (all three of them) works? it did for me

sudo systemctl stop xcash-daemon &>/dev/null
sleep 5
sudo systemctl start xcash-daemon &>/dev/null
sleep 30s

to

sudo systemctl stop xcash-daemon &>/dev/null
sleep 30s
sudo systemctl start xcash-daemon &>/dev/null
sleep 30s
n3me5is-git commented 4 years ago

@zachhildreth I'm trying to debug the script... However, just a question: with the local daemon modifications, is the re-import after wallet creation still necessary? What was exactly the purpose of that code? The comment of that code is

  # import the wallet if they created the wallet before. This should fix any 0 balance error

What's this "0 balance error"? And why re-creating the wallet with import from seed is solving the issue? Because we are 1) creating the wallet (and so scanning the whole BC), 2) synching the wallet and 3) deleting the wallet and reimporting it using the seed extracted from the old wallet file, opening it. So with this flow, the sync procedure (2) it seems useless, if we recreate the wallet just after...

I'm just asking, because I don't know it the re-import is really needed... And if needed, what about using a "rescan blockchain" + "refresh" RPC call inside the sync_wallet function directly instead?

When we simply import the wallet, we are doing the import and then the sync. So in any case, the sync_wallet function should be called after the re-import (if the maintain that code)

n3me5is-git commented 4 years ago

Also, the sync_xcash_wallet function is doing the same things (more or less) of the get_current_xcash_wallet_data called just after. Both the functions are waiting for the wallet to sync, so the operation is done 2 times... We could remove the sync_xcash_wallet and keep only the other

zachhildreth commented 4 years ago

The import wallet is needed as its a legacy bug of some CN coins, where it will show 0 balance for a "wallet file" that is created sometimes. rescan_bc does not fix it from the testing, but this was a long time ago when we were building the GUI 1.0 wallet. The import fixes it, and is what we do in the GUI wallet as well when someone creates a wallet.

n3me5is-git commented 4 years ago

Ok, now I understand. So the legacy bug is still there and we are doing this only as a "security procedure" to cover also this case. If I understood correctly, the 0 balance will persist also when receiving incoming TXs (or blocks)? Because when we create a new wallet it will be initially with 0 balance.

xcash-rocks commented 4 years ago

sudo systemctl stop xcash-daemon &>/dev/null sleep 30s sudo systemctl start xcash-daemon &>/dev/null sleep 30s

This worked as now the "X-CASH Wallet Data" gets displayed!

Still, the script crashed during install():"Importing X-CASH Wallet", while deleting the delegate-wallet.

Changed Line 1863 & 1965: rm "${XCASH_DPOPS_INSTALLATION_DIR}"xcash-wallets/delegate-wallet* 2&> /dev/null to rm -f "${XCASH_DPOPS_INSTALLATION_DIR}"xcash-wallets/delegate-wallet* 2&> /dev/null

Voilà! It works :-)!

@n3me5is-git can you update your PR; will retest and hopefully close this issue afterwards.

n3me5is-git commented 4 years ago

Ok, I will update it tomorrow! I'm going to sleep now! I will test the import too! However I can't understand why a sleep 30s after the service stop can solve the bug. Systemctl usually waits until the process exits, so 5s vs 30s should have the same impact...

I will test with -f when removing the wallet. Also this is strange, because if you before didn't find the wallet files after the script crash, it's because the rm command executed correctly. It's strange that this time the installer complained about deleting the wallet...

But if this solves the issue, good!

xcash-rocks commented 4 years ago

I guess it was a chain of bugs

n3me5is-git commented 4 years ago

For example now it worked with rm -f and with sleep 5s... I will update everything tomorrow after another couple of tests!

n3me5is-git commented 4 years ago

@xcash-rocks Can you test this last commit added to the PR? https://github.com/X-CASH-official/xcash-dpops/pull/41/commits/66fe81022d16b8099d6f796cfbf4a7f6d2f7b203 Let me know if everything is better now

n3me5is-git commented 4 years ago

@xcash-rocks @zachhildreth Testing now the installation (with root, because I'm inside the LXC container, and without deleting the old blockchain otherwise it takes ages to test). Can you confirm that after the new PR everything works and is smoother? Maybe we converged... And also optimized the script. The configuration with a new wallet works now in my case, so probably also the installation (the code is the same, for the wallet creation/import parts)

n3me5is-git commented 4 years ago

I conform that installation worked during last test (root - generate new wallet/password/key)

xcash-rocks commented 4 years ago

66fe810

Confirming successful installation and configuration!