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

refactor: split tari_base_node and tari_console_wallet into a lib component #4818

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

mrnaveira
Copy link
Contributor

Still a work in progress

Description

  • Splitted tari_base_node and tari_console_wallet into lib.rs and main.rs
  • TBD: split tari_miner

Motivation and Context

To enable the new cucumber-rs integration tests on tari-dan#103, specifically to spawn base nodes and wallets, it's desirable to be able to treat them as native Rust functions exported in a lib.rs module.

How Has This Been Tested?

@mrnaveira mrnaveira changed the title refactor: Split tari_base_node and tari_console_wallet into a lib component refactor: split tari_base_node and tari_console_wallet into a lib component Oct 18, 2022
@CjS77 CjS77 added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Oct 18, 2022
@@ -20,12 +20,15 @@
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#![allow(dead_code, unused)]
Copy link
Member

@sdbondi sdbondi Oct 19, 2022

Choose a reason for hiding this comment

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

I think the problem here is that we have a lib and a bin in the same crate.
The bin is including the modules directly e.g. mod automation and also uses the lib use tari_console_wallet::***. The former bin modules are built into the bin but are unused.

I think we'll need to expose the modules via the tari_console_wallet lib and have the main/bin use that.

@CjS77 CjS77 removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 19, 2022
@stringhandler
Copy link
Collaborator

I'm going to merge it for now. We can remove the clippy allows later

@stringhandler stringhandler merged commit 2dbceaa into tari-project:feature-dan Oct 19, 2022
sdbondi added a commit to sdbondi/tari that referenced this pull request Nov 14, 2022
* feature-dan: (21 commits)
  fix(core)!: remove unused get_committees call from base node (tari-project#4880)
  fix: correct value for validator_node_timeout consensus constant in localnet (tari-project#4879)
  feat: add block height to input request to get network consensus constants (tari-project#4856)
  fix: remove unused config for validator node (tari-project#4849)
  feat: add missing fields to grpc consensus constants interface (tari-project#4845)
  chore: merge development into feature-dan (tari-project#4815)
  refactor: split tari_base_node and tari_console_wallet into a lib component (tari-project#4818)
  fix(core)!: adds utxo and block info to get_template_registrations request (tari-project#4789)
  fix: computation of vn mmr (tari-project#4772)
  fix(wallet/grpc): add transaction id and template_address to template_reg response (tari-project#4788)
  fix: fix get shard key (tari-project#4744)
  feat(core): store and fetch templates from lmdb (tari-project#4726)
  fix: fix validator node registration logic (tari-project#4718)
  feat(base_node_grpc_client): add getActiveValidatorNodes method (tari-project#4719)
  fix after merge
  fix(core): bring validator node MR inline with other merkle root code (tari-project#4692)
  feat(core): add validator registration sidechain feature (tari-project#4690)
  fix merge issues
  feat: add grpc to get shard key for public key (tari-project#4654)
  feat: add validator node registration (tari-project#4507)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants