-
Notifications
You must be signed in to change notification settings - Fork 386
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
AuRa specific changes for themerge #4124
Conversation
Co-authored-by: Nikita Mescheryakov <root@nmescheryakov.com>
* Add test to BlockTreeTests.cs * Add sync test scenarios to BlockTreeTests.cs Co-authored-by: hrugved <whrugved@gmail.com>
# Conflicts: # src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/NewPayloadV1Handler.cs
|
||
namespace Nethermind.Api.Extensions; | ||
|
||
// Assemblies containing instances of this interface will be the ones |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use documenting comments: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/documentation-comments
|
||
if (TrySetState(parent.StateRoot)) | ||
{ | ||
Block? processed = ProcessPreparedBlock(block, null); | ||
if (processed != null) | ||
block = processed; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we processing block here?
That looks weird to me, this is not the place for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to process here to apply rewards even on empty blocks
src/Nethermind/Nethermind.Merge.Gnosis/InitializationSteps/InitializeBlockchainAuRaMerge.cs
Outdated
Show resolved
Hide resolved
@@ -21,6 +21,6 @@ namespace Nethermind.Api.Extensions; | |||
|
|||
public interface IPluginConfig : IConfig | |||
{ | |||
[ConfigItem(Description = "Order of plugin initialization", DefaultValue = "[Clique, Aura, Ethash, Merge, MEV, HealthChecks, Hive]")] | |||
[ConfigItem(Description = "Order of plugin initialization", DefaultValue = "[Clique, Aura, Ethash, AuRaMerge, Merge, MEV, HealthChecks, Hive]")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please document in AuraMergePlugin that it has to go before the Merge plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a note in AuRaMergePlugin
Changes:
Types of changes
What types of changes does your code introduce?
Put an
x
in the boxes that applyTesting
Requires testing
In case you checked yes, did you write tests??