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

Explicitly enable or disable Stratum in config file (Issue 9785) #10521

Merged
merged 5 commits into from
Apr 2, 2019

Conversation

lamafab
Copy link
Contributor

@lamafab lamafab commented Mar 25, 2019

Hi

This PR adds the requested feature from issue #9785.

Changes

  • Users can now directly enable or disable Stratum in the configuration file:
[stratum]
disable = false
port = 8007
secret = "Yellow"
[stratum]
disable = true
  • For backward compatibility reasons, Stratum is enabled by default if the [stratum] section is present and not disabled by setting disable = true.
  • Contains additional tests for the Stratum section in the config file.

I've run those changes directly on my machine; it works as intended (assuming these are the expected requirements. IMO, it would be cleaner if Stratum was only enabled if we explicitly set a enabled = true option. This would break backward compatibility, however).

Any feedback is welcome.

@parity-cla-bot
Copy link

It looks like @lamafab signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@lamafab
Copy link
Contributor Author

lamafab commented Mar 25, 2019

Are the CPP errors in your CI related to my changes? It does look like another problem, unless I'm wrong.

Scanning dependencies of target libparity
[ 10%] Creating directories for 'libparity'
[ 20%] No download step for 'libparity'
[ 30%] No update step for 'libparity'
[ 40%] No patch step for 'libparity'
[ 50%] No configure step for 'libparity'
[ 60%] Performing build step for 'libparity'
CMake Error at /builds/parity/parity-ethereum/parity-clib/examples/cpp/build/libparity-prefix/src/libparity-stamp/libparity-build-.cmake:16 (message):
  Command failed: 1

   '/usr/bin/cmake' '-Dmake=' '-Dconfig=' '-P' '/builds/parity/parity-ethereum/parity-clib/examples/cpp/build/libparity-prefix/src/libparity-stamp/libparity-build--impl.cmake'

  See also

    /builds/parity/parity-ethereum/parity-clib/examples/cpp/build/libparity-prefix/src/libparity-stamp/libparity-build-*.log


CMakeFiles/libparity.dir/build.make:110: recipe for target 'libparity-prefix/src/libparity-stamp/libparity-build' failed
make[2]: *** [libparity-prefix/src/libparity-stamp/libparity-build] Error 1
CMakeFiles/Makefile2:67: recipe for target 'CMakeFiles/libparity.dir/all' failed
make[1]: *** [CMakeFiles/libparity.dir/all] Error 2
make: *** [all] Error 2
Makefile:83: recipe for target 'all' failed

@soc1c soc1c added this to the 2.5 milestone Mar 25, 2019
@soc1c soc1c added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. M2-config 📂 Chain specifications and node configurations. labels Mar 25, 2019
@niklasad1
Copy link
Collaborator

/cc @TriplEight any idea what's going on here?

I think we should enable VERBOSE build for the cpp example (I have no clue what's is going on)

make VERBOSE=1 -j $THREADS

@TriplEight
Copy link
Collaborator

@niklasad1 here you go: #10524

@soc1c
Copy link
Contributor

soc1c commented Mar 26, 2019

I just restarted the job and it worked, maybe hold back with hacking the pipelines and try restarting it in future? ;)

@soc1c soc1c modified the milestones: 2.5, 2.6 Apr 2, 2019
@HCastano HCastano added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 2, 2019
@soc1c soc1c added the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Apr 2, 2019
@soc1c soc1c merged commit 288d737 into openethereum:master Apr 2, 2019
@HCastano
Copy link
Contributor

HCastano commented Apr 2, 2019

We should also update the wiki to reflect this change. @lamafab do you mind putting in a PR here: https://github.com/paritytech/wiki?

@lamafab
Copy link
Contributor Author

lamafab commented Apr 2, 2019

@HCastano Sure, no problem. I will do that after work (this evening).

ordian added a commit that referenced this pull request Apr 5, 2019
* master:
  fix(light cull): poll light cull instead of timer (#10559)
  Update Issue Template to direct security issue to email (#10562)
  RPC: Implements eth_subscribe("syncing") (#10311)
  Explicitly enable or disable Stratum in config file (Issue 9785) (#10521)
  version: bump master to 2.6 (#10560)
  tx-pool: check transaction readiness before replacing (#10526)
  fix(light account response): update `tx_queue` (#10545)
  Update light client harcoded headers (#10547)
  fix(light eth_gasPrice): ask network if not in cache (#10535)
  Implement caching for service transactions checker (#10088)
  build android with cache, win fixes (#10546)
  clique: make state backfill time measurement more accurate (#10551)
  updated lru-cache to 0.1.2 (#10542)
@stone212
Copy link

stone212 commented Oct 28, 2019

@lamafab You said Stratum is enabled by default if the [stratum] section is present and not disabled by setting disable = true

Does this mean:

Stratum is

  1. enabled by default if the [stratum] section is present
  2. not disabled by setting disable = true

Why would you have disable = true if it does not disable it?

Maybe you mean:

"Stratum is enabled by default if [stratum] section is present, unless disabled by setting disable = true"

?

@lamafab
Copy link
Contributor Author

lamafab commented Oct 28, 2019

@stone212 This decision was made for backwards compatibility reasons. Stratum is enabled if the [stratum] section is present, either with or without disable = false. If you set disable = true, then Stratum gets disabled.

Sorry for the confusion...

@stone212
Copy link

@lamafab It's a good decision, I just wanted to make the language clear. Your sentence could mean one thing or the exact opposite. Good to have this change!

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. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M2-config 📂 Chain specifications and node configurations. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants