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

[Mining Node] Add mining node to distribution #2649

Merged

Conversation

StriderDM
Copy link
Contributor

@StriderDM StriderDM commented Feb 16, 2021

Description

Disable toggle_mining command in the base node.
Add mining node to startup scripts.
Update scripts to start mining node instead of using the enable_mining flag.
Add mining node to distribution/installer for osx, linux, mac.

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Feature refactor (No new feature or functional changes, but performance or technical debt improvements)
  • New Tests
  • Documentation

Checklist:

  • I'm merging against the development branch.
  • I ran cargo-fmt --all before pushing.
  • I ran cargo test successfully before submitting my PR.
  • I have squashed my commits into a single commit.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

Disable toggle_mining command in the base node.
Add mining node to startup scripts.
Update scripts to start mining node instead of using the enable_mining flag.
Add mining node to distribution/installer for osx, linux, mac.
@hansieodendaal
Copy link
Contributor

Looking good reviewing the changes; will run a full installer/tarball creation with installation test for Windows and Ubuntu.

Notes:

  • If you could possibly solve the Ubuntu start scripts to verify if Tor is running that would be awesome; the previous attempt did not work properly for both the on-click-execute-all and single execution options and was removed a while back.
  • I assume the standalone miner shortcut's icon is also pointing to this? Source: "tari_logo_purple.ico"; DestDir: "{userdocs}\..\temp\tari_icons";

Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@stringhandler stringhandler merged commit baf3464 into tari-project:development Feb 16, 2021
@leet4tari
Copy link
Contributor

A little late to comment on this, but there are shell scripts that have been includes as binary files ... Now double checking and see them as empty files? applications/tari_mining_node/osx/start_tari_mining_node.sh, applications/tari_mining_node/ubuntu/start_tari_mining_node.sh, applications/tari_mining_node/windows/start_tari_console_wallet.lnk ... I think they were symlinks, but something is not quite right there?

@StriderDM
Copy link
Contributor Author

@leet4tari They're not empty files. The two start_tari_mining_node.sh files are aliases and the start_tari_console_wallet.lnk is a windows shortcut. If you open them with an editor like Atom do they still display as being empty?

@StriderDM
Copy link
Contributor Author

@hansieodendaal For your notes. Didn't know the first point was an issue on ubuntu. For the second point it should be, didn't change anything regarding the shortcuts, used what already existed for that.

@leet4tari
Copy link
Contributor

Looking in GitHub - https://github.com/StriderDM/tari/blob/21b50bc29e43c148284a52c9e55c8023aa9601d5/applications/tari_mining_node/osx/start_tari_mining_node.sh is empty/binary and not a symbolic link ... Alias is what I think Microsoft calls it?
Screenshot 2021-02-16 at 14 12 41

verse https://github.com/StriderDM/tari/blob/21b50bc29e43c148284a52c9e55c8023aa9601d5/applications/tari_base_node/osx/start_all
Screenshot 2021-02-16 at 14 12 57

I can't say anything about the Windows Alias to the .lnk file.

@StriderDM
Copy link
Contributor Author

@leet4tari Fixed here:
#2652

Credit to @hansieodendaal

@leet4tari
Copy link
Contributor

Still looks wrong in GitHub
Screenshot 2021-02-17 at 08 36 50

Did a git clone and looks like symlinks on my OSX. Maybe just some sort of glitch in GitHub.

Not tested the scripts myself.

@leet4tari
Copy link
Contributor

Looked up git usage and symlinks ... Not sure why we using the symlinks, but might be an idea either to agree on and how they used. Reading https://stackoverflow.com/questions/954560/how-does-git-handle-symbolic-links seems like a can of worms we could avoid.

@StriderDM
Copy link
Contributor Author

@leet4tari I have seen that stackoverflow link. You can also have a look here https://mokacoding.com/blog/symliks-in-git/

They were created with the following:

ln -s ./runtime/start_tari_mining_node.sh start_tari_mining_node

@leet4tari
Copy link
Contributor

Still looks broken in the web view on GitHub and both the articles show a folder with an arrow, that is what I expect from a symlink, but our repo shows a bin file, which we can't open using a browser via the web. I am just pointing this out, mostly to keep in mind.

Could be how Windows deals with symlinks or maybe a change to git.core settings. This is just a warning.

stringhandler added a commit that referenced this pull request Feb 17, 2021
Changes since v0.8.2

Base Node
---
- [#2635](#2635) [base-node] Add user agent to peer display info (#2635)
- [#2626](#2626) [base-node] Add duplicate input_output tx validator

Wallet
---
- [#2656](#2656) [wallet] Fixed timelocked balance
- [#2632](#2632) [wallet] Update TXO validation protocol to use RPC interface to base node
- [#2624](#2624) [wallet] Set confirmations required via libwallet and config
- [#2644](#2644) [wallet] Add unique constraint to commitment in outputs table
- [#2637](#2637) [wallet] Update Transaction Receiver protocol to persist transaction earlier
- [#2625](#2625) [wallet] Remove the Tor identity getter from the FFI
- [#2614](#2614) [wallet] New Transaction Validation protocol for Wallet
- [#2619](#2619) [wallet] Add tx sending mechanism to wallet

Mining Node
---
- [#2652](#2652) [mining-node] Recreated symlinks without extension
- [#2649](#2649) [mining-node] Add mining node to distribution

Merge Mining Proxy
---
- [#2631](#2631) [mmproxy] Fix missing Content-Type header in some json responses
- [#2616](#2616) [merge-mining] submit_block returns OK if block was submitted for tari or monero

Other
---
- [#2655](#2655) [tests] Remove cancelled output excluded test
- [#2647](#2647) [tests] Minor improvements to DHT logging
- [#2653](#2653) [tests] Speed up `Transaction Info` Cucumber test
- [#2643](#2643) [common] Dont propagate messages we have already
- [#2612](#2612) [ci] Optimize integration tests
- [#2651](#2651) [tests] Fix num confirmations test backend
- [#2641](#2641) [common] Retry peers that have been marked as offline after a length of time
- [#2650](#2650) [ci] Allow CI to use latest mdbook
- [#2627](#2627) [tests] Double spend test for mempool transaction selection
- [#2623](#2623) [chore] Add extra logging to help investigate fee mismatch
- [#2630](#2630) [common] Shorter ban for RPC negotiation timeout in header sync, RPC timeout configurable
- [#2629](#2629) [chore] Clean up duplicate check
- [#2628](#2628) [tests] Add test_find_duplicate_input
- [#2648](#2648) [ci] Run cargo test with verbose output
- [#2638](#2638) [common] Ban peers that flood messages
- [#2636](#2636) [explorer] Add a nodejs block explorer
- [#2634](#2634) [fix] Dont store messages of a banned peer
- [#2613](#2613) [tests] Add unit tests for lock heights
stringhandler added a commit that referenced this pull request Feb 17, 2021
Changes since v0.8.2

Base Node
---
- [#2635](#2635) [base-node] Add user agent to peer display info (#2635)
- [#2626](#2626) [base-node] Add duplicate input_output tx validator

Wallet
---
- [#2656](#2656) [wallet] Fixed timelocked balance
- [#2632](#2632) [wallet] Update TXO validation protocol to use RPC interface to base node
- [#2624](#2624) [wallet] Set confirmations required via libwallet and config
- [#2644](#2644) [wallet] Add unique constraint to commitment in outputs table
- [#2637](#2637) [wallet] Update Transaction Receiver protocol to persist transaction earlier
- [#2625](#2625) [wallet] Remove the Tor identity getter from the FFI
- [#2614](#2614) [wallet] New Transaction Validation protocol for Wallet
- [#2619](#2619) [wallet] Add tx sending mechanism to wallet

Mining Node
---
- [#2652](#2652) [mining-node] Recreated symlinks without extension
- [#2649](#2649) [mining-node] Add mining node to distribution

Merge Mining Proxy
---
- [#2631](#2631) [mmproxy] Fix missing Content-Type header in some json responses
- [#2616](#2616) [merge-mining] submit_block returns OK if block was submitted for tari or monero

Other
---
- [#2655](#2655) [tests] Remove cancelled output excluded test
- [#2647](#2647) [tests] Minor improvements to DHT logging
- [#2653](#2653) [tests] Speed up `Transaction Info` Cucumber test
- [#2643](#2643) [common] Dont propagate messages we have already
- [#2612](#2612) [ci] Optimize integration tests
- [#2651](#2651) [tests] Fix num confirmations test backend
- [#2641](#2641) [common] Retry peers that have been marked as offline after a length of time
- [#2650](#2650) [ci] Allow CI to use latest mdbook
- [#2627](#2627) [tests] Double spend test for mempool transaction selection
- [#2623](#2623) [chore] Add extra logging to help investigate fee mismatch
- [#2630](#2630) [common] Shorter ban for RPC negotiation timeout in header sync, RPC timeout configurable
- [#2629](#2629) [chore] Clean up duplicate check
- [#2628](#2628) [tests] Add test_find_duplicate_input
- [#2648](#2648) [ci] Run cargo test with verbose output
- [#2638](#2638) [common] Ban peers that flood messages
- [#2636](#2636) [explorer] Add a nodejs block explorer
- [#2634](#2634) [fix] Dont store messages of a banned peer
- [#2613](#2613) [tests] Add unit tests for lock heights
@stringhandler stringhandler mentioned this pull request Feb 17, 2021
stringhandler added a commit that referenced this pull request Feb 17, 2021
Changes since v0.8.2

Base Node
---
- [#2635](#2635) [base-node] Add user agent to peer display info (#2635)
- [#2626](#2626) [base-node] Add duplicate input_output tx validator

Wallet
---
- [#2656](#2656) [wallet] Fixed timelocked balance
- [#2632](#2632) [wallet] Update TXO validation protocol to use RPC interface to base node
- [#2624](#2624) [wallet] Set confirmations required via libwallet and config
- [#2644](#2644) [wallet] Add unique constraint to commitment in outputs table
- [#2637](#2637) [wallet] Update Transaction Receiver protocol to persist transaction earlier
- [#2625](#2625) [wallet] Remove the Tor identity getter from the FFI
- [#2614](#2614) [wallet] New Transaction Validation protocol for Wallet
- [#2619](#2619) [wallet] Add tx sending mechanism to wallet

Mining Node
---
- [#2652](#2652) [mining-node] Recreated symlinks without extension
- [#2649](#2649) [mining-node] Add mining node to distribution

Merge Mining Proxy
---
- [#2631](#2631) [mmproxy] Fix missing Content-Type header in some json responses
- [#2616](#2616) [merge-mining] submit_block returns OK if block was submitted for tari or monero

Other
---
- [#2655](#2655) [tests] Remove cancelled output excluded test
- [#2647](#2647) [tests] Minor improvements to DHT logging
- [#2653](#2653) [tests] Speed up `Transaction Info` Cucumber test
- [#2643](#2643) [common] Dont propagate messages we have already
- [#2612](#2612) [ci] Optimize integration tests
- [#2651](#2651) [tests] Fix num confirmations test backend
- [#2641](#2641) [common] Retry peers that have been marked as offline after a length of time
- [#2650](#2650) [ci] Allow CI to use latest mdbook
- [#2627](#2627) [tests] Double spend test for mempool transaction selection
- [#2623](#2623) [chore] Add extra logging to help investigate fee mismatch
- [#2630](#2630) [common] Shorter ban for RPC negotiation timeout in header sync, RPC timeout configurable
- [#2629](#2629) [chore] Clean up duplicate check
- [#2628](#2628) [tests] Add test_find_duplicate_input
- [#2648](#2648) [ci] Run cargo test with verbose output
- [#2638](#2638) [common] Ban peers that flood messages
- [#2636](#2636) [explorer] Add a nodejs block explorer
- [#2634](#2634) [fix] Dont store messages of a banned peer
- [#2613](#2613) [tests] Add unit tests for lock heights
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

4 participants