Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Initialize private tx logger only if private tx functionality is enabled #10758

Merged
merged 3 commits into from
Jun 19, 2019

Conversation

grbIzl
Copy link
Collaborator

@grbIzl grbIzl commented Jun 18, 2019

Private tx logger tried to open log file even for the case, when private tx functionality was disabled.

Closes #10726

@grbIzl grbIzl added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jun 18, 2019
@grbIzl grbIzl requested a review from dvdplm June 18, 2019 08:32
@ordian ordian added this to the 2.6 milestone Jun 18, 2019
Copy link
Collaborator

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Should it be backported or was it introduced recently?

@ordian
Copy link
Collaborator

ordian commented Jun 18, 2019

test failure seems legit

@grbIzl
Copy link
Collaborator Author

grbIzl commented Jun 18, 2019

Should it be backported or was it introduced recently?

It was introduced in #10056 As far as I can seee, it was not released yet

Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

looks ok, consider adding impl Default for ProviderConfig

@@ -1458,7 +1461,7 @@ mod tests {
private_provider_conf: ProviderConfig {
validator_accounts: Default::default(),
signer_account: Default::default(),
logs_path: Some(Directories::default().base),
logs_path: None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like ProviderConfig should impl Default too?

@dvdplm dvdplm added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 18, 2019
@grbIzl grbIzl added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Jun 19, 2019
@niklasad1 niklasad1 merged commit e1333ea into master Jun 19, 2019
@niklasad1 niklasad1 deleted the RemovePrivateLogErrorMessage branch June 19, 2019 14:28
dvdplm added a commit that referenced this pull request Jun 24, 2019
…anager

* master:
  Print warnings when using dangerous settings for ValidatorSet (#10733)
  ethcore/res: activate atlantis classic hf on block 8772000 (#10766)
  refactor: Fix indentation (#10740)
  Updated Bn128PairingImpl to use optimized batch pairing  (#10765)
  fix: aura don't add `SystemTime::now()` (#10720)
  Initialize private tx logger only if private tx functionality is enabled (#10758)
  Remove unused code (#10762)
  Remove calls to heapsize (#10432)
dvdplm added a commit that referenced this pull request Jun 25, 2019
…dp/fix/prevent-building-block-on-top-of-same-parent

* dp/chore/aura-log-validator-set-in-epoch-manager:
  remove dead code
  Treat empty account the same as non-exist accounts in EIP-1052 (#10775)
  docs: Update Readme with TOC, Contributor Guideline. Update Cargo package descriptions (#10652)
  cleanup
  On second thought non-validators are allowed to report
  Move Engine::register_client to be before other I/O handler registration (#10767)
  cleanup
  Print warnings when using dangerous settings for ValidatorSet (#10733)
  ethcore/res: activate atlantis classic hf on block 8772000 (#10766)
  refactor: Fix indentation (#10740)
  Updated Bn128PairingImpl to use optimized batch pairing  (#10765)
  fix: aura don't add `SystemTime::now()` (#10720)
  Initialize private tx logger only if private tx functionality is enabled (#10758)
  Remove unused code (#10762)
  Remove calls to heapsize (#10432)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve startup message "Cannot read logs…"
4 participants