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

ctxless cli node utils #3652

Merged
merged 5 commits into from
Aug 16, 2024
Merged

ctxless cli node utils #3652

merged 5 commits into from
Aug 16, 2024

Conversation

tzemanovic
Copy link
Member

Describe your changes

Do not load Context for node utils (test-genesis) cmds

Checklist before merging

  • If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
    • This will require 2 reviewers to approve the changes

@tzemanovic tzemanovic changed the title ctxless node utils ctxless cli node utils Aug 15, 2024
@tzemanovic tzemanovic marked this pull request as ready for review August 15, 2024 10:44
@tzemanovic tzemanovic requested a review from batconjurer August 15, 2024 10:44
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 61.19%. Comparing base (de6f4cc) to head (9bba8fd).
Report is 118 commits behind head on main.

Files Patch % Lines
crates/node/src/shell/testing/client.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3652      +/-   ##
==========================================
+ Coverage   61.18%   61.19%   +0.01%     
==========================================
  Files         312      312              
  Lines      101353   101351       -2     
==========================================
+ Hits        62011    62022      +11     
+ Misses      39342    39329      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tzemanovic
Copy link
Member Author

I'm having some issues with the test-genesis cmd - it crashes on spawning the dummy task:

The application panicked (crashed).
Message:  there is no reactor running, must be called from the context of a Tokio 1.x runtime
Location: crates/node/src/lib.rs:983

@tzemanovic
Copy link
Member Author

I'm having some issues with the test-genesis cmd - it crashes on spawning the dummy task:

The application panicked (crashed).
Message:  there is no reactor running, must be called from the context of a Tokio 1.x runtime
Location: crates/node/src/lib.rs:983

I fixed it by removing the dummy task and also added an e2e test to cover this

@tzemanovic tzemanovic force-pushed the tomas/ctxless-node-utils branch from 15f899e to 9bba8fd Compare August 16, 2024 11:27
@tzemanovic tzemanovic added the merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass label Aug 16, 2024
@Fraccaman Fraccaman requested a review from sug0 August 16, 2024 12:36
Copy link
Collaborator

@sug0 sug0 left a comment

Choose a reason for hiding this comment

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

LGTM

mergify bot added a commit that referenced this pull request Aug 16, 2024
@mergify mergify bot merged commit a3c30dd into main Aug 16, 2024
19 checks passed
@mergify mergify bot deleted the tomas/ctxless-node-utils branch August 16, 2024 13:16
Fraccaman added a commit that referenced this pull request Aug 26, 2024
…dump

* origin/main: (191 commits)
  ci: use custom runner for docs step
  fix: better checkout settings, fix sccache wrapper
  changelog: add #3694
  fix wasm dir during join-network
  Changelog #3689
  Minor fixes to governance cli/msgs
  fix: use current epoch
  Changelog
  Add script boot localnet with two genesis validators
  Updates expected msg in wallet e2e test
  Changelog #3681
  Two attempts for decryption password
  Removes comments
  Prompts the key that we are trying to decrypt
  Terminates client if decryption of signing key fails
  Propagates error in sdk if decryption of signing key fails
  Removes FIXME comment
  Add fullnode support
  added changelog
  token: fix transfer to self
  make build: exclude fuzz targets
  replace context with client
  Changelog #3676
  Removes test step from wasm for test Makefile
  Removes dev dependencies from wasm tests
  Amend comment in genesis sign txs
  Changelog #3669
  Updates help message of `--gas-limit`
  Refactors `dispatch_inner_txs`
  Adds masp fee payment integration test
  Improves safety of masp txs events
  fix for clippy
  is_apple_silicon
  fixup! ci: add job checking for cargo docs
  README: update docs section
  fix rustdoc issues
  ci: do not run changelog check on rc branches
  ci: increase integration tests timeout
  ci: add job checking for cargo docs
  Namada 0.43.0
  generate key
  changelog: add #3445
  fuzz: add txs_wasm_run
  fuzz: add txs_finalize_block target
  add conditional fuzzing sig acceptance at lower level
  fuzz: add txs_process_proposal
  fuzz: add README.md
  token: avoid trying to read denom when fuzzing
  prepare_proposal: make tx fee checks pass for fuzzing
  fuzz: add txs_prepare_proposal
  fuzz: use a shorter target name
  fix clippy
  tx: prevent overflow in arbitrary masp builder
  fuzz_txs_mempool: rm unused import
  impl Arbitrary for MaspBuilder to derive tx sections
  use updated jubjub that prevents invalid arb ExtendedPoint
  fuzz_txs_mempool: catch panics in serialization
  fuzz_txs_mempool: use non-panicking tx serialization method
  tx: add non-panicking serialization method
  tx: add arbitrary masp tx section
  make/fuzz-txs-mempool: run with `--dev` to avoid OOM
  shell/mempool: skip chain ID check for fuzzing
  tx: fix manual impl Arbitrary for Section
  core/key: fix arbitrary impls
  fuzz_txs_mempool: re-use TestShell between runs
  fuzz: set dev opt-level = 3
  node/init_chain: no wasm pre-compile for fuzzing
  Makefile: add `cargo-fuzz` dep and `fuzz-txs-mempool` recipe
  fuzz mempool with arb txs
  tx: impl Arbitrary for Tx
  init a fuzz target with cargo-fuzz
  fixup! test/e2e/ledger: add test-genesis `--check-can-sign` args
  Improves failures handling in `try_masp_fee_payment`
  Matches masp ref order in protocol with that of the masp vp
  changelog: add #3660
  test/e2e/ledger: add test-genesis `--check-can-sign` args
  cli: added `--check-can-sign` arg to `namadan utils test-genesis`
  add changelog
  rm accidentally revived files
  test/e2e/ledger_tests: add localnet genesis test using `utils test-genesis`
  fix to check the existing addr
  fix default refund target
  fix tests
  add shielded mode to transfer context
  change to transparent addr
  Added changelog
  Fixed bug in creating BenchShieldedContext
  Changelog for #3655
  Optimize building chain ctx
  node/utils/test-genesis: remove unnecessary broadcaster task
  Added tests that birthdays work correctly with shielded sync
  changelog: add #3652
  cli/client: fix clippy
  cli/node: don't load ctx for node utils
  Added dated keys to shielded sync args
  Removed birthday logic from payment address generation
  Moved type alias to a more logical location
  Changelog #3626
  Changes gaia e2e tests to use masp internal address instead of payment addresses for ibc-shielding
  Reverts previous changes
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants