Skip to content

Conversation

@dannasessha
Copy link
Contributor

Working toward #17 - For now this is just a README update.

If you'd like to see the abstract path setup or specific naming of what I'm about to build into the tool this is a good moment to comment.

Problem:
I couldn't get the zcashd to write to the debug.log file at all. I saw this issue on zcash's page, which is listed as a 'good first issue' but it's a bit beyond me, at least at a glance, wasn't sure if it could be a lead on this issue but I'd like another dev to confirm this isn't working for them, or point out what I've done wrong.

Maybe there's other logging or metrics you'd like to capture @zancas ? I was thinking about trying to timestamp log files, for example.

I'm concerned about these kinds of issues too, so I'd like to have those kinds of updates(or reassurance I shouldn't worry!) in place before moving around more Rust.

@dannasessha dannasessha added the DRAFT DRAFT / WIP label Jul 2, 2022
@dannasessha dannasessha requested a review from zancas July 2, 2022 12:15
@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2022

Codecov Report

Merging #26 (3db9367) into dev (ddf8261) will decrease coverage by 1.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##              dev      #26      +/-   ##
==========================================
- Coverage   58.22%   57.20%   -1.02%     
==========================================
  Files          30       30              
  Lines        5838     5942     +104     
==========================================
  Hits         3399     3399              
- Misses       2439     2543     +104     
Impacted Files Coverage Δ
cli/src/lib.rs 0.00% <ø> (ø)
cli/src/main.rs 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddf8261...3db9367. Read the comment docs.

@dannasessha
Copy link
Contributor Author

In conversation: "the regtest specific information should go in a README.md inside the regtest directory"

@dannasessha
Copy link
Contributor Author

In further conversation "dump all binaries in regtest/bin"

@dannasessha
Copy link
Contributor Author

I now don't like the base regtest directory, which I thought of shortly after working on it again today...

...now there's /zingolib/regtest/datadir/zcash/regtest/ and /zingolib/regtest/datadir/lightwalletd/db/regtest/ ...

I'd like to change that to something unique. Also, I know regtest is the feature, but it strikes me that with zcashd and lightwalletd making "regtest" specific subdirectories, maybe we should consider having this be an overall ... modes subdirectory, which could use testnet and mainnet too? Provide a harness for that... I see it's a bit beyond the scope right now but keeping an eye out for extensibility. RFC.

@dannasessha
Copy link
Contributor Author

Still also:
-I'm not getting zcashd to log the way I want or expect.
-LWD's YAML file includes zcash-conf-path but I'm not sure it's actually doing anything.

@dannasessha dannasessha changed the title WIP: readme includes updated manual regtest mode setup WIP: zingo-cli regtest mode invocation Jul 5, 2022
@dannasessha
Copy link
Contributor Author

Overall, this is looking really promising. Please refactor such that there are no typehints.

This is done, along with some other stuff too. I'd say all the core functions are in place now, but the code should be cleaned up and maybe minimized, as well as probably moved to another mod.

@zancas
Copy link
Member

zancas commented Jul 7, 2022

This is coming along!!

Please add logic to handle graceful shut-down of spawned threads.

When the user instructs the zingo-cli to shutdown it must:

  • announce that it was instructed to shutdown
  • issue proper shutdown commands to each spawned process
  • report progress to the user
  • monitor the behavior of its spawned processes
  • report success-or-failure of spawn shutdowns
  • shut itself down

zancas
zancas previously approved these changes Jul 7, 2022
Copy link
Member

@zancas zancas left a comment

Choose a reason for hiding this comment

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

This is a significant, functional piece of work. It should be merged.

@zancas zancas removed the DRAFT DRAFT / WIP label Jul 7, 2022
@zancas
Copy link
Member

zancas commented Jul 7, 2022

I removed the DRAFT label. There are next steps, but this is a coherent chunk, and should be landed.

Copy link
Member

@zancas zancas left a comment

Choose a reason for hiding this comment

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

This is a coherent chunk.

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.

4 participants