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

SecondsPerSlot move to BlocksConfig #4944

Conversation

smartprogrammer93
Copy link
Contributor

@smartprogrammer93 smartprogrammer93 commented Nov 29, 2022

Resolves #4871

Changes:

  • SecondsPerSlot moved to BlocksConfig while deprecating it in MergeConfig.
  • Passed HiveTest

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • Yes
  • No

In case you checked yes, did you write tests??

  • Yes
  • No

@smartprogrammer93 smartprogrammer93 force-pushed the fix/slotTime_to_be_in_init_config_instead_of_merge branch 2 times, most recently from d8214bd to 6c965ea Compare November 29, 2022 15:55
@smartprogrammer93 smartprogrammer93 marked this pull request as ready for review November 29, 2022 15:57
@@ -7,6 +7,7 @@
using Nethermind.Consensus.Transactions;
using Nethermind.Core;
using Nethermind.Core.Specs;
using Nethermind.Evm;
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats with these new using(s)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to move IBlocksConfig to EVM project

Copy link
Member

Choose a reason for hiding this comment

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

Do we have to? Maybe we don't have to pass IBlockCofnig everywhere, just the actual SecondsPerBlock? Do we need IBLockConfig at all then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't the idea of the Configs that they can be accessed where needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made more sense to put it inside the config project since it is not related to the EVM so i moved it there.

Type type = config.GetType();
Type interfaceType = type.GetInterface($"I{type.Name}");
PropertyInfo propertyInfo = interfaceType.GetProperty(propertyName);
ConfigItemAttribute attribute = propertyInfo.GetCustomAttribute<ConfigItemAttribute>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be, but i tried it, and it did not work.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LukaszRozmej i did, it still did not return the attribute. i even went and specified that the attributed is inherited at its definition but still it did not work

@smartprogrammer93 smartprogrammer93 force-pushed the fix/slotTime_to_be_in_init_config_instead_of_merge branch from ba215bc to ebc1808 Compare December 7, 2022 22:09
@rubo
Copy link
Contributor

rubo commented Dec 12, 2022

@MarekM25 Any comments on this?


namespace Nethermind.Consensus
namespace Nethermind.Config
Copy link
Contributor

Choose a reason for hiding this comment

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

Why BlocksConfig moved to Nethermind.Config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it is needed in projects that do not reference Consensus

Copy link
Member

Choose a reason for hiding this comment

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

Isn't IBlockConfig sufficient for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LukaszRozmej It is, but i dont see why we would move the interface without moving the implementation class

{
public static T GetDefaultValue<T>(this IConfig config, string propertyName)
{
Type type = config.GetType();
Copy link
Contributor

Choose a reason for hiding this comment

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

test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@codecov
Copy link

codecov bot commented Dec 28, 2022

Codecov Report

❗ No coverage uploaded for pull request base (fix/forkId_calculation_is_wrong_after_timestamp_activation_fix@25f696a). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 0f0a443 differs from pull request most recent head dc6dfdd. Consider uploading reports for the commit dc6dfdd to get more accurate results

Additional details and impacted files
@@                                        Coverage Diff                                        @@
##             fix/forkId_calculation_is_wrong_after_timestamp_activation_fix    #4944   +/-   ##
=================================================================================================
  Coverage                                                                  ?   66.75%           
=================================================================================================
  Files                                                                     ?     1967           
  Lines                                                                     ?    70327           
  Branches                                                                  ?    13085           
=================================================================================================
  Hits                                                                      ?    46950           
  Misses                                                                    ?    19407           
  Partials                                                                  ?     3970           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

…after_timestamp_activation_fix' into fix/slotTime_to_be_in_init_config_instead_of_merge

namespace Nethermind.Consensus
namespace Nethermind.Config
Copy link
Member

Choose a reason for hiding this comment

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

Isn't IBlockConfig sufficient for this?

Type type = config.GetType();
Type interfaceType = type.GetInterface($"I{type.Name}");
PropertyInfo propertyInfo = interfaceType.GetProperty(propertyName);
ConfigItemAttribute attribute = propertyInfo.GetCustomAttribute<ConfigItemAttribute>();
Copy link
Member

Choose a reason for hiding this comment

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

src/Nethermind/Nethermind.Merge.Plugin/MergePlugin.cs Outdated Show resolved Hide resolved
}
else if (_mergeConfig.SecondsPerSlot == defaultValue)
{
_mergeConfig.SecondsPerSlot = _blocksConfig.SecondsPerSlot;
Copy link
Member

Choose a reason for hiding this comment

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

I find this going both ways somewhat disturbing. MergeConfig.SecondsPerSlot shouldn't be used anymore, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct, but i followed the same think we are doing with with MiningConfig and BlockConfig . you can check it here.
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.Init/Steps/MigrateConfigs.cs

@smartprogrammer93
Copy link
Contributor Author

Merging since already added tests and got enough approvals. will adjust in the target branch if any is needed.

@smartprogrammer93 smartprogrammer93 merged commit f0d2f6a into fix/forkId_calculation_is_wrong_after_timestamp_activation_fix Jan 1, 2023
@smartprogrammer93 smartprogrammer93 deleted the fix/slotTime_to_be_in_init_config_instead_of_merge branch January 1, 2023 18:35
MarekM25 added a commit that referenced this pull request Jan 11, 2023
* More test fixes

* Fix for init and meter

* Make withdrawals decoding optional

* include 4895 in chainspecParameters

* introduce WithdrawalTimestamp

* Fix `PayloadAttributes` handling when null

* Fix withdrawals length calculation

* Refactor and add checks for withdrawals for RPC methods of v1

* Add engine_getPayloadV2

* Format whitespaces

* Refactor and remove redundant "V1"

* started adding Enginge V2 tests - V2_processing_block_should_serialize_valid_responses

* formatting

* formatting

* Implement withdrawal root validation

* working on tests and fixes

* fix block for processing

* BlockValidator WithdrawalRoot

* Fix withdrawal trie proof validation

* add WithdrawalApplier

* Remove redundant withdrawals hash check

* Remove withdrawal check for null

* Add withdrawals test chain spec

* Reformat whitespaces

* add withdrawalApplier to blockchainProcessor

* add withdrawals to ExecutionPayload

* test fixes

* fix EVM.test

* fix EthereumTests

* fix Benchmarks.sln

* Revise withdrawals representation according to the spec and refactor

* Rename `IWithdrawalApplier` to `IWithdrawalProcessor`

* Add tests for withdrawal encoding/decoding

* Rename `Recipient` to `Address`

* Reformat whitespaces

* Revise withdrawals length calculation in block encoding

* HasBody?

* Remove redundant withdrawals hash check

* fix withdrawals_test chainspec && added IReleaseSpec.WithdrawalsEnabled

* Fixes

* fix GenesisLoader

* fix extra-data

* Refactor withdrawal validation by implementing `IWithdrawalValidator`

* Fix failing tests

* Reformat whitespaces

* fix withdrawalsTimestamp

* temporary change validation

* null handling?

* Applying Marek's suggestion. Not sure about this one

* Fix Ethereum tests

* Fix test cases

* Fix benchmark build

* Fix payload attributes validation

* Remove "V1" from `IForkchoiceUpdatedHandler` name

* Update tests

* hack AuRa tests for now

* fix more tests

* fix more tests

* fix build

* + fix one more tests

* fix Synchronization tests

* Update tests to 0aa59689101f64cab8fa1526d9cf6e647ddba946

* fixed withdrawal chainspec

* withdrawals block validator tests

* Implement Engine API tests

* Reformat code

* fix chainspec?

* Fix withdrawals decoding

* fix chainspec timestamp

* Add and fix Engine API tests

* Revise block decoder tests

* Revise file headers

* Revise file headers

* Add missing file headers

* fix AuRa test

* adjusting comments

* load genesis tests

* Lukasz suggestions

* formating

* Fix build

* withdrawals_hivetests.json + cosmetic

* Add more unit tests

* spacing

* Try on fixing timestamp activation with same value as genesis timestamp

* Fix gnosis and chiado ForkId Calculations

* fix tests

* Cleanup and more tests

* Janky but working solution to very rare edge case?

* Fix flakiness of caused by Parallelizable

* adding engine tests

* working on more tests

* add loop in test

* Can_apply_withdrawals_correctly test

* more test cases

* Update withdrawals hive tests configuration

* Refactor and fix some null reference warnings

* Expose withdrawals to JSON-RPC modules

* Fix broken tests

* Revise `ForkchoiceUpdatedHandler` string output

* fix Can_apply_withdrawals_correctly

* fix whitespaces

* fix whitespaces

* more whitespaces fixes

* work on Withdrawals_transition test cases

* adjust CustomSpecProvider

* Introduce IEip1995Spec

* small changes in tests

* Fix license

* add comments

* Apply Marek Suggestion

* Adjust Lic to Rubo

* Marek Suggestions

* Fix

* fix withdrawals in ChainLevelHelper

* fix hive sync tests

* fix?

* Revise withdrawals root hash encoding/decoding and its tests

* fix PayloadAttributes ToString

* add more temp logs to investigate hive tests

* more logs

* revert not needed logs

* fix test

* ignore incorrect tests

* Fix tests broken by withdrawals decoding revision

* Revise file headers

* Final appraoch, removed GetSpec complexity

* Forgotten changes

* SecondsPerSlot to BlocksConfig

* Minor + config changes

* Typo fix

* remove empty line

* Move BlocksConfig to Nethermind.Config project

* Benchmark build fix

* Rename `ExecutionPayloadV1` to `ExecutionPayloadV2`

* Revise withdrawals check

* Refactor tests

* Optimize withdrawals root hash decoding in block header

* Implements ForkId tests that are ub EIP-6122

* Spacing

* Fix build

* Fix ForkId Test case

* Removing 3675 and using MergeForkId Transition

* Final test fix

* Fix tests

* Rename `ExecutionPayloadV2` to `ExecutionPayload`

* add one more line in Nlog (temp)

* Revert NLog for jsonRpc

* fix missing body?

* adjust TxPool logs

* Add more test cases for `BlockBodiesMessageSerializer`

* Add tests for `BlockHeader.HasBody`

* Revise `null` handling for `BlockHeader.HasBody`

* fix BlockBody empty

* cosmetic

* fix CI

* Revise tests

* Update null checks with pattern matching

* Refactor block body initialization and add tests

* Revise whitespaces

* cosmetic

* ForkId calculation polish (#5068)

* Refactor - mainly ChainSpecBasedSpecProvider

* fixes

* Better warning message

* Simplify sanity check

* one more simplification

* Revise Clique block production according to withdrawals rules

* Add withdrawals to `eth_getBlockByNumber` tests

* SecondsPerSlot move to BlocksConfig (#4944)

Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Resolves #4871

* fix build

* removed duplicated NSubstitute

* Add missing file headers

* resolved some review comments

* more review comments

* more review related changes

* fix Benchmarks build

* Refactor tests

* refactor tests - review comment

* cosmetic

* Revise suggested block validation messages

* Refactor patricia tries

* fix InvalidBlockInterceptor for Withdrawals

* fix whitespaces

* fix tests

Co-authored-by: smartprogrammer <smartprogrammer@windowslive.com>
Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Ahmad Bitar <33181301+smartprogrammer93@users.noreply.github.com>
LukaszRozmej added a commit that referenced this pull request Jan 20, 2023
* Add engine_getPayloadV2

* Format whitespaces

* Refactor and remove redundant "V1"

* started adding Enginge V2 tests - V2_processing_block_should_serialize_valid_responses

* formatting

* formatting

* Implement withdrawal root validation

* working on tests and fixes

* fix block for processing

* BlockValidator WithdrawalRoot

* Fix withdrawal trie proof validation

* add WithdrawalApplier

* Remove redundant withdrawals hash check

* Remove withdrawal check for null

* Add withdrawals test chain spec

* Reformat whitespaces

* add withdrawalApplier to blockchainProcessor

* add withdrawals to ExecutionPayload

* test fixes

* fix EVM.test

* fix EthereumTests

* fix Benchmarks.sln

* Revise withdrawals representation according to the spec and refactor

* Rename `IWithdrawalApplier` to `IWithdrawalProcessor`

* Add tests for withdrawal encoding/decoding

* Rename `Recipient` to `Address`

* Reformat whitespaces

* Revise withdrawals length calculation in block encoding

* HasBody?

* Remove redundant withdrawals hash check

* fix withdrawals_test chainspec && added IReleaseSpec.WithdrawalsEnabled

* Fixes

* fix GenesisLoader

* fix extra-data

* Refactor withdrawal validation by implementing `IWithdrawalValidator`

* Fix failing tests

* Reformat whitespaces

* fix withdrawalsTimestamp

* temporary change validation

* null handling?

* Applying Marek's suggestion. Not sure about this one

* Fix Ethereum tests

* Fix test cases

* Fix benchmark build

* Fix payload attributes validation

* Remove "V1" from `IForkchoiceUpdatedHandler` name

* Update tests

* hack AuRa tests for now

* fix more tests

* fix more tests

* fix build

* + fix one more tests

* fix Synchronization tests

* Update tests to 0aa59689101f64cab8fa1526d9cf6e647ddba946

* fixed withdrawal chainspec

* withdrawals block validator tests

* Implement Engine API tests

* Reformat code

* fix chainspec?

* Fix withdrawals decoding

* fix chainspec timestamp

* Add and fix Engine API tests

* Revise block decoder tests

* Revise file headers

* Revise file headers

* Add missing file headers

* fix AuRa test

* adjusting comments

* load genesis tests

* Lukasz suggestions

* formating

* Fix build

* withdrawals_hivetests.json + cosmetic

* Add more unit tests

* spacing

* Try on fixing timestamp activation with same value as genesis timestamp

* Fix gnosis and chiado ForkId Calculations

* fix tests

* Cleanup and more tests

* Janky but working solution to very rare edge case?

* Fix flakiness of caused by Parallelizable

* Correct usage of chain ID and network ID

* adding engine tests

* working on more tests

* add loop in test

* Can_apply_withdrawals_correctly test

* more test cases

* Update withdrawals hive tests configuration

* Refactor and fix some null reference warnings

* Expose withdrawals to JSON-RPC modules

* Fix broken tests

* Revise `ForkchoiceUpdatedHandler` string output

* fix Can_apply_withdrawals_correctly

* fix whitespaces

* fix whitespaces

* more whitespaces fixes

* add more tests

* Fix chainId loading from spec

* Rename for clarity

* work on Withdrawals_transition test cases

* adjust CustomSpecProvider

* Introduce IEip1995Spec

* Make test NetworkId vs ChainId different

Changing ChainId to another number breaks a lot of test, that are based to hashes and RLP. Let at least test if the values differ

* small changes in tests

* Fix license

* add comments

* Apply Marek Suggestion

* Adjust Lic to Rubo

* Marek Suggestions

* Fix

* fix withdrawals in ChainLevelHelper

* Fix chainid for account abstraction; add and improve tests; fix a tutorial link

* Fix naming in other projects, whitespaces

* Fix more renaming

* fix hive sync tests

* fix?

* Revise withdrawals root hash encoding/decoding and its tests

* fix PayloadAttributes ToString

* add more temp logs to investigate hive tests

* more logs

* revert not needed logs

* fix test

* ignore incorrect tests

* Fix tests broken by withdrawals decoding revision

* Revise file headers

* Final appraoch, removed GetSpec complexity

* Forgotten changes

* SecondsPerSlot to BlocksConfig

* Minor + config changes

* Typo fix

* remove empty line

* Move BlocksConfig to Nethermind.Config project

* Benchmark build fix

* Rename `ExecutionPayloadV1` to `ExecutionPayloadV2`

* Revise withdrawals check

* Refactor tests

* Optimize withdrawals root hash decoding in block header

* Implements ForkId tests that are ub EIP-6122

* Spacing

* Fix build

* Fix ForkId Test case

* Removing 3675 and using MergeForkId Transition

* Final test fix

* Fix tests

* Rename `ExecutionPayloadV2` to `ExecutionPayload`

* add one more line in Nlog (temp)

* Revert NLog for jsonRpc

* fix missing body?

* adjust TxPool logs

* Add more test cases for `BlockBodiesMessageSerializer`

* Add tests for `BlockHeader.HasBody`

* Revise `null` handling for `BlockHeader.HasBody`

* fix BlockBody empty

* cosmetic

* fix CI

* Revise tests

* Update null checks with pattern matching

* Refactor block body initialization and add tests

* Revise whitespaces

* cosmetic

* ForkId calculation polish (#5068)

* Refactor - mainly ChainSpecBasedSpecProvider

* fixes

* Better warning message

* Simplify sanity check

* one more simplification

* Revise Clique block production according to withdrawals rules

* Add withdrawals to `eth_getBlockByNumber` tests

* SecondsPerSlot move to BlocksConfig (#4944)

Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Resolves #4871

* fix build

* removed duplicated NSubstitute

* Fix network id in node info

* Rollback spec change

* Rename NetworkId class to BlockchainIds; more renaming and usage of TestBlockChainIds

* Improve a test

* Fix chainid/networkid usage

* Refactors

* Improve CustomSpecProvider ctor

* Use test provider

* ethstats fix

* whitespace

* Fix

Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com>
Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: smartprogrammer <smartprogrammer@windowslive.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Ahmad Bitar <33181301+smartprogrammer93@users.noreply.github.com>
flcl42 added a commit that referenced this pull request Jan 21, 2023
…4867)

* add WithdrawalApplier

* Remove redundant withdrawals hash check

* Remove withdrawal check for null

* Add withdrawals test chain spec

* Reformat whitespaces

* add withdrawalApplier to blockchainProcessor

* add withdrawals to ExecutionPayload

* test fixes

* fix EVM.test

* fix EthereumTests

* fix Benchmarks.sln

* Revise withdrawals representation according to the spec and refactor

* Rename `IWithdrawalApplier` to `IWithdrawalProcessor`

* Add tests for withdrawal encoding/decoding

* Rename `Recipient` to `Address`

* Reformat whitespaces

* Revise withdrawals length calculation in block encoding

* HasBody?

* Remove redundant withdrawals hash check

* fix withdrawals_test chainspec && added IReleaseSpec.WithdrawalsEnabled

* Fixes

* fix GenesisLoader

* fix extra-data

* Refactor withdrawal validation by implementing `IWithdrawalValidator`

* Fix failing tests

* Reformat whitespaces

* fix withdrawalsTimestamp

* temporary change validation

* null handling?

* Applying Marek's suggestion. Not sure about this one

* Fix Ethereum tests

* Fix test cases

* Fix benchmark build

* Fix payload attributes validation

* Remove "V1" from `IForkchoiceUpdatedHandler` name

* Update tests

* hack AuRa tests for now

* fix more tests

* fix more tests

* fix build

* + fix one more tests

* fix Synchronization tests

* Update tests to 0aa59689101f64cab8fa1526d9cf6e647ddba946

* fixed withdrawal chainspec

* withdrawals block validator tests

* Implement Engine API tests

* Reformat code

* fix chainspec?

* Fix withdrawals decoding

* fix chainspec timestamp

* Add and fix Engine API tests

* Revise block decoder tests

* Revise file headers

* Revise file headers

* Add missing file headers

* fix AuRa test

* adjusting comments

* load genesis tests

* Lukasz suggestions

* formating

* Fix build

* withdrawals_hivetests.json + cosmetic

* Add more unit tests

* spacing

* Try on fixing timestamp activation with same value as genesis timestamp

* Fix gnosis and chiado ForkId Calculations

* fix tests

* Cleanup and more tests

* Janky but working solution to very rare edge case?

* Fix flakiness of caused by Parallelizable

* Correct usage of chain ID and network ID

* adding engine tests

* working on more tests

* add loop in test

* Can_apply_withdrawals_correctly test

* more test cases

* Update withdrawals hive tests configuration

* Refactor and fix some null reference warnings

* Expose withdrawals to JSON-RPC modules

* Fix broken tests

* Revise `ForkchoiceUpdatedHandler` string output

* fix Can_apply_withdrawals_correctly

* fix whitespaces

* fix whitespaces

* more whitespaces fixes

* add more tests

* Fix chainId loading from spec

* Rename for clarity

* work on Withdrawals_transition test cases

* adjust CustomSpecProvider

* Introduce IEip1995Spec

* Make test NetworkId vs ChainId different

Changing ChainId to another number breaks a lot of test, that are based to hashes and RLP. Let at least test if the values differ

* small changes in tests

* Fix license

* add comments

* Apply Marek Suggestion

* Adjust Lic to Rubo

* Marek Suggestions

* Fix

* fix withdrawals in ChainLevelHelper

* Fix chainid for account abstraction; add and improve tests; fix a tutorial link

* Fix naming in other projects, whitespaces

* Fix more renaming

* fix hive sync tests

* fix?

* Revise withdrawals root hash encoding/decoding and its tests

* fix PayloadAttributes ToString

* add more temp logs to investigate hive tests

* more logs

* revert not needed logs

* fix test

* ignore incorrect tests

* Fix tests broken by withdrawals decoding revision

* Revise file headers

* Final appraoch, removed GetSpec complexity

* Forgotten changes

* SecondsPerSlot to BlocksConfig

* Minor + config changes

* Typo fix

* remove empty line

* Move BlocksConfig to Nethermind.Config project

* Benchmark build fix

* Rename `ExecutionPayloadV1` to `ExecutionPayloadV2`

* Revise withdrawals check

* Refactor tests

* Optimize withdrawals root hash decoding in block header

* Add fields, spec, header encoding

* Implements ForkId tests that are ub EIP-6122

* Spacing

* Fix build

* Fix ForkId Test case

* Removing 3675 and using MergeForkId Transition

* Final test fix

* Fix tests

* Rename `ExecutionPayloadV2` to `ExecutionPayload`

* add one more line in Nlog (temp)

* Revert NLog for jsonRpc

* fix missing body?

* adjust TxPool logs

* Add more test cases for `BlockBodiesMessageSerializer`

* Add tests for `BlockHeader.HasBody`

* Revise `null` handling for `BlockHeader.HasBody`

* fix BlockBody empty

* cosmetic

* fix CI

* Revise tests

* Update null checks with pattern matching

* Refactor block body initialization and add tests

* Revise whitespaces

* cosmetic

* ForkId calculation polish (#5068)

* Refactor - mainly ChainSpecBasedSpecProvider

* fixes

* Better warning message

* Simplify sanity check

* one more simplification

* Revise Clique block production according to withdrawals rules

* Add withdrawals to `eth_getBlockByNumber` tests

* SecondsPerSlot move to BlocksConfig (#4944)

Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Resolves #4871

* fix build

* removed duplicated NSubstitute

* Fix network id in node info

* Rollback spec change

* Rename NetworkId class to BlockchainIds; more renaming and usage of TestBlockChainIds

* Improve a test

* Fix chainid/networkid usage

* Refactors

* Improve CustomSpecProvider ctor

* Use test provider

* ethstats fix

* whitespace

* Fix

* Fix header encoding: loading from genesis

* Remove fork, simplify codfe, fix merge conflicts

* Improve tests, fix spaces

* Fix whitespaces

* Fix tests

* Fix space

* Fix Rlp for headers

Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com>
Co-authored-by: smartprogrammer <smartprogrammer@windowslive.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Ahmad Bitar <33181301+smartprogrammer93@users.noreply.github.com>
flcl42 added a commit that referenced this pull request Jan 21, 2023
* Reformat whitespaces

* add withdrawalApplier to blockchainProcessor

* add withdrawals to ExecutionPayload

* test fixes

* fix EVM.test

* fix EthereumTests

* fix Benchmarks.sln

* Revise withdrawals representation according to the spec and refactor

* Rename `IWithdrawalApplier` to `IWithdrawalProcessor`

* Add tests for withdrawal encoding/decoding

* Rename `Recipient` to `Address`

* Reformat whitespaces

* Revise withdrawals length calculation in block encoding

* HasBody?

* Remove redundant withdrawals hash check

* fix withdrawals_test chainspec && added IReleaseSpec.WithdrawalsEnabled

* Fixes

* fix GenesisLoader

* fix extra-data

* Refactor withdrawal validation by implementing `IWithdrawalValidator`

* Fix failing tests

* Reformat whitespaces

* fix withdrawalsTimestamp

* temporary change validation

* null handling?

* Applying Marek's suggestion. Not sure about this one

* Fix Ethereum tests

* Fix test cases

* Fix benchmark build

* Fix payload attributes validation

* Remove "V1" from `IForkchoiceUpdatedHandler` name

* Update tests

* hack AuRa tests for now

* fix more tests

* fix more tests

* fix build

* + fix one more tests

* fix Synchronization tests

* Update tests to 0aa59689101f64cab8fa1526d9cf6e647ddba946

* fixed withdrawal chainspec

* withdrawals block validator tests

* Implement Engine API tests

* Reformat code

* fix chainspec?

* Fix withdrawals decoding

* fix chainspec timestamp

* Add and fix Engine API tests

* Revise block decoder tests

* Revise file headers

* Revise file headers

* Add missing file headers

* fix AuRa test

* adjusting comments

* load genesis tests

* Lukasz suggestions

* formating

* Fix build

* withdrawals_hivetests.json + cosmetic

* Add more unit tests

* spacing

* Try on fixing timestamp activation with same value as genesis timestamp

* Fix gnosis and chiado ForkId Calculations

* fix tests

* Cleanup and more tests

* Janky but working solution to very rare edge case?

* Fix flakiness of caused by Parallelizable

* Correct usage of chain ID and network ID

* adding engine tests

* working on more tests

* add loop in test

* Can_apply_withdrawals_correctly test

* more test cases

* Update withdrawals hive tests configuration

* Refactor and fix some null reference warnings

* Expose withdrawals to JSON-RPC modules

* Fix broken tests

* Revise `ForkchoiceUpdatedHandler` string output

* fix Can_apply_withdrawals_correctly

* fix whitespaces

* fix whitespaces

* more whitespaces fixes

* add more tests

* Fix chainId loading from spec

* Rename for clarity

* work on Withdrawals_transition test cases

* adjust CustomSpecProvider

* Introduce IEip1995Spec

* Make test NetworkId vs ChainId different

Changing ChainId to another number breaks a lot of test, that are based to hashes and RLP. Let at least test if the values differ

* small changes in tests

* Fix license

* add comments

* Apply Marek Suggestion

* Adjust Lic to Rubo

* Marek Suggestions

* Fix

* fix withdrawals in ChainLevelHelper

* Fix chainid for account abstraction; add and improve tests; fix a tutorial link

* Fix naming in other projects, whitespaces

* Fix more renaming

* fix hive sync tests

* fix?

* Revise withdrawals root hash encoding/decoding and its tests

* fix PayloadAttributes ToString

* add more temp logs to investigate hive tests

* more logs

* revert not needed logs

* fix test

* ignore incorrect tests

* Fix tests broken by withdrawals decoding revision

* Revise file headers

* Final appraoch, removed GetSpec complexity

* Forgotten changes

* SecondsPerSlot to BlocksConfig

* Minor + config changes

* Typo fix

* remove empty line

* Move BlocksConfig to Nethermind.Config project

* Benchmark build fix

* Rename `ExecutionPayloadV1` to `ExecutionPayloadV2`

* Revise withdrawals check

* Refactor tests

* Optimize withdrawals root hash decoding in block header

* Add fields, spec, header encoding

* Implements ForkId tests that are ub EIP-6122

* Spacing

* Fix build

* Fix ForkId Test case

* Removing 3675 and using MergeForkId Transition

* Final test fix

* Fix tests

* Rename `ExecutionPayloadV2` to `ExecutionPayload`

* add one more line in Nlog (temp)

* Revert NLog for jsonRpc

* fix missing body?

* adjust TxPool logs

* Add more test cases for `BlockBodiesMessageSerializer`

* Add tests for `BlockHeader.HasBody`

* Revise `null` handling for `BlockHeader.HasBody`

* fix BlockBody empty

* cosmetic

* fix CI

* Revise tests

* Update null checks with pattern matching

* Refactor block body initialization and add tests

* Revise whitespaces

* cosmetic

* ForkId calculation polish (#5068)

* Refactor - mainly ChainSpecBasedSpecProvider

* fixes

* Better warning message

* Simplify sanity check

* one more simplification

* Revise Clique block production according to withdrawals rules

* Add withdrawals to `eth_getBlockByNumber` tests

* SecondsPerSlot move to BlocksConfig (#4944)

Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Resolves #4871

* fix build

* removed duplicated NSubstitute

* Fix network id in node info

* Rollback spec change

* Rename NetworkId class to BlockchainIds; more renaming and usage of TestBlockChainIds

* Improve a test

* Fix chainid/networkid usage

* Refactors

* Improve CustomSpecProvider ctor

* Use test provider

* ethstats fix

* whitespace

* Fix

* Fix header encoding: loading from genesis

* Remove fork, simplify codfe, fix merge conflicts

* Improve tests, fix spaces

* Fix whitespaces

* Fix tests

* Fix space

* Fix Rlp for headers

* Add DATAHASH opcode

* LogFinder should be initialized befre UserOperationPools

* Fix postmerge

* Add no blobs for benchmarks, let's add it with DATAHASH benchmarks

Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com>
Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: smartprogrammer <smartprogrammer@windowslive.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Ahmad Bitar <33181301+smartprogrammer93@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants