Allow for programmatic configuration of protocol settings#845
Allow for programmatic configuration of protocol settings#845erikzhang merged 4 commits intoneo-project:masterfrom devhawk:devhawk/protocol-settings
Conversation
There was a problem hiding this comment.
Such a flexible and well designed PR, @devhawk.
Great job with specific use of C# functions...aehuahuea
As soon as we have NEO 3.0 first preview we can validate this in practice.
PS: I restarted the TRAVIS job because UT looks pass locally as well.
|
This is a great Pull Request @devhawk |
|
I can't imagine how to switch networks without downtime after a node is running. |
| IConfigurationSection section = new ConfigurationBuilder().AddJsonFile("protocol.json", true).Build().GetSection("ProtocolConfiguration"); | ||
| Default = new ProtocolSettings(section); | ||
| var settings = new ProtocolSettings(configuration.GetSection("ProtocolConfiguration")); | ||
| return null == Interlocked.CompareExchange(ref _default, settings, null); |
There was a problem hiding this comment.
When it return true? only when change the first time?
There was a problem hiding this comment.
Yes, UpdateDefault will only return true the first time it was called. CompareExchange atomically compares _default to null and stores settings in _default if and only if the comparison succeeds.
I agree with @erikzhang - a node can't switch networks once it is up and running. This code ensures _default's value can only be written once - even if there are multiple threads in the process accessing ProtocolSettings.Default.
Codecov Report
@@ Coverage Diff @@
## master #845 +/- ##
==========================================
+ Coverage 38.39% 38.45% +0.05%
==========================================
Files 176 176
Lines 12465 12475 +10
==========================================
+ Hits 4786 4797 +11
+ Misses 7679 7678 -1
Continue to review full report at Codecov.
|
| return null == Interlocked.CompareExchange(ref _default, settings, null); | ||
| } | ||
|
|
||
| public static bool Initialize(IConfiguration configuration) |
There was a problem hiding this comment.
Do we need this Initialize()? UpdateDefault() will be called automatically when accessing Default.
There was a problem hiding this comment.
Initialize is a public method intended to be called by host code. UpdateDefault is a private helper method that does the actual CompareExchange so that I didn't Duplicate code in the Initialize and Default getter code paths.
There was a problem hiding this comment.
This is very useful for testing. This way we can manage to set everything up (using Initialize() before actually starting the other components. We could apply a similar solution to neo-plugins Settings: https://github.com/neo-project/neo-plugins/blob/a72c44813271efcafe0a4fcee2505fae3a3b4716/ApplicationLogs/Settings.cs#L9
igormcoelho
left a comment
There was a problem hiding this comment.
Fine for me. As long as it's used once, and before network initialization.
Tests pass, but in the future we need to test this in a more integrated way (perhaps the DI approach @rodoufu).
* 修复该页面在 GitHub 上显示的格式错误 * Update consensus.md
This PR adds a mechanism to configure the protocol settings programmatically. This enhancement is needed to enable automatic configuration of the privatenet blockchain,
This change adds an Initialize static function to ProtocolSettings, enabling the host process to pass custom protocol configuration information without needing to write a protocol.json file to disk. ASP.NET Configuration already supports multiple configuration providers. The caller of Initialize is free to use whatever configuration provider they wish, including in-memory .NET objects.
If Initialize method is not called, the first time the ProtocolSettings.Default get function is called, protocol configuration is loaded exactly as it loaded today - the protocol.json file is used as a configuration provider if available with mainnet defaults used for any configuration settings not provided. Again, as today these settings are cached for later use.
ProtocolSettings.Initialize must be called before the ProtocolSettings.Default property is ever accessed. Initialize will not overwrite existing loaded protocol settings - regardless if those settings were loaded by an earlier call to Initialize or by the first time the Default property is accessed. CompareExchange is used to ensure protocol settings are only initialized once, even in the face of multiple threads possibly accessing ProtocolSettings.
PR also includes unit tests for ProtocolSettings initialization. All unit tests pass locally.