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.

Sign up for GitHub

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

Jump to bottom

[BESU-200] Xpruning-blocks-retained should accept an integer in the config. #440

Merged
merged 2 commits into from
Mar 3, 2020
Merged

[BESU-200] Xpruning-blocks-retained should accept an integer in the config. #440

merged 2 commits into from
Mar 3, 2020

Conversation

AbdelStark
Copy link
Contributor

PR description

From the original description of the Jira ticket BESU-200, reporter @ajsutton :
When specifying the Xpruning-blocks-retained option via a config file,

Xpruning-blocks-retained=10000

results in the error
TomlInvalidTypeException: Value of 'Xpruning-blocks-retained' is a integer while processing argument at or before arg[-1] '?' in [--config-file=/etc/besu/config.toml]: org.apache.tuweni.toml.TomlInvalidTypeException: Value of 'Xpruning-blocks-retained' is a integer

And you have to instead use:

Xpruning-blocks-retained="10000" 

Since the value is an integer it would be good if providing an integer worked. This would also be a useful option to make "official" instead of under the -X category.  Ethereum 2 nodes need more than the default 1000 blocks of history so changing this configuration will become more common as ETH2 progresses.

Probability: Probable
Severity: Moderate

Changes

  • Removed -X unstable prefix for pruning options.
  • Moved --pruning-blocks-retainedand --pruning-block-confirmations in BesuCommand.
  • Deleted PrunerOptions class, directly use the pruning options as fields in BesuCommand instead.
  • Updated types of pruning options from long to Integer, this change enables to use the integer value in the config file.
  • Updated tests accordingly.

Documentation changes required

  • Add documentation for --pruning-blocks-retainedand --pruning-block-confirmations options.

Fixed Issue(s)

Fixes https://jira.hyperledger.org/browse/BESU-200

Signed-off-by: Abdelhamid Bakhta abdelhamid.bakhta@consensys.net

@AbdelStark
Xpruning-blocks-retained should accept an integer in the config.
9d3321a 
Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
@AbdelStark AbdelStark added bug Something isn't working enhancement New feature or request doc-change-required Indicates an issue or PR that requires doc to be updated labels Mar 3, 2020
@AbdelStark AbdelStark self-assigned this Mar 3, 2020
@AbdelStark
Merge remote-tracking branch 'upstream/master' into besu-200/pruning-…
faa8377 
…blocks-retained-cli-option

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
@AbdelStark AbdelStark merged commit b9e0b3b into hyperledger:master Mar 3, 2020
@AbdelStark AbdelStark deleted the besu-200/pruning-blocks-retained-cli-option branch March 4, 2020 08:07
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Mar 12, 2020
@AbdelStark
Added changelog entries for PR:
ba37fd0 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
@AbdelStark AbdelStark mentioned this pull request Mar 12, 2020
Merged
AbdelStark added a commit that referenced this pull request Mar 12, 2020
@AbdelStark
Added changelog entries for PR: (#459)
dfc7ece 
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
@MadelineMurray MadelineMurray mentioned this pull request Mar 12, 2020
Closed
@grantnoble grantnoble mentioned this pull request Mar 13, 2020
Merged
AbdelStark added a commit that referenced this pull request Apr 2, 2020
@AbdelStark
[EIP-1559] Step 0 - Add CLI command line flag --Xeip1559-enabled. (#641)
0e9add5 
* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Add ExperimentalEIPs class to hold static CLI guard flags preventing users from accidentally enabling unwanted EIPs.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Remove unrelevant test

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 3, 2020
@AbdelStark
[EIP-1559] Step 1 - Apply block structure modification (#619)
2148116 
* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* ### Description
BlockHeader object needs to be modified in order to add the new field (baseFee) as specified in the EIP.
We should take care about the RLP encoding/decoding of this structure since it has to include or not the new fields depending on whether we are pre fork or post fork.

- Update `core.BlockHeader.java`
    - Add `baseFee`
    - Update `readFrom` method for RLP decoding
    - Update `writeTo` method for RLP encoding
- Update `plugin.data.BlockHeader.java`
    - Add `getBaseFee` method

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* TODO Add CLI command line flag `--Xeip1559-enabled`.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* TODO Add CLI command line flag `--Xeip1559-enabled`.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Added unstable annotation for getBaseFee. Moved from long to `Optional<Long>`

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Spotless apply

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Fixed error

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* fix plugin API hash

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* RLP encoding / decoding operations are guarded with the feature flag.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 3, 2020
@AbdelStark
[EIP-1559] Step 2 - Apply transaction structure modification (#620)
0680f5e 
* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* ### Description
BlockHeader object needs to be modified in order to add the new field (baseFee) as specified in the EIP.
We should take care about the RLP encoding/decoding of this structure since it has to include or not the new fields depending on whether we are pre fork or post fork.

- Update `core.BlockHeader.java`
    - Add `baseFee`
    - Update `readFrom` method for RLP decoding
    - Update `writeTo` method for RLP encoding
- Update `plugin.data.BlockHeader.java`
    - Add `getBaseFee` method

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* ### Description
Transaction object needs to be modified in order to add the 2 new fields (`gasPremium` and `feeCap`) as specified in the EIP.
We should take care about the RLP encoding/decoding of this structure since it has to include or not the new fields depending on whether we are pre fork or post fork.

- Update core `Transaction` object
    - Add gasPremium and feeCap fields
    - Update readFrom method for RLP decoding
    - Update writeTo method for RLP encoding
- Update plugin `Transaction` interface
    - Add `getGasPremium` and `getFeeCap` methods

This EIP introduces gasPremium and feeCap fields in the transaction. They need to be included in the signing mechanism.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* TODO Add CLI command line flag `--Xeip1559-enabled`.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Remove TODO

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Merge step 0 and make EIP-1559 specific fields optional
Add feature flag guard for RLP encoding / decoding of transactions

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* fix error

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* update plugin api known hash

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 3, 2020
@AbdelStark
[EIP-1559] Step 3 - Implement EIP-1559 logic in a manager class (#622)
96122e0 
* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* ### Description
BlockHeader object needs to be modified in order to add the new field (baseFee) as specified in the EIP.
We should take care about the RLP encoding/decoding of this structure since it has to include or not the new fields depending on whether we are pre fork or post fork.

- Update `core.BlockHeader.java`
    - Add `baseFee`
    - Update `readFrom` method for RLP decoding
    - Update `writeTo` method for RLP encoding
- Update `plugin.data.BlockHeader.java`
    - Add `getBaseFee` method

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* ### Description
Transaction object needs to be modified in order to add the 2 new fields (`gasPremium` and `feeCap`) as specified in the EIP.
We should take care about the RLP encoding/decoding of this structure since it has to include or not the new fields depending on whether we are pre fork or post fork.

- Update core `Transaction` object
    - Add gasPremium and feeCap fields
    - Update readFrom method for RLP decoding
    - Update writeTo method for RLP encoding
- Update plugin `Transaction` interface
    - Add `getGasPremium` and `getFeeCap` methods

This EIP introduces gasPremium and feeCap fields in the transaction. They need to be included in the signing mechanism.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* We need a manager class to hold EIP-1559 core logic. This helper class will be then used by glue code. This separation make testing simpler.
Specifically the following methods:

- `long computeBaseFee(final long parentBaseFee, final long parentGasUsed)`
- `boolean isValidBaseFee(final long parentBaseFee, final long baseFee)`
- `long eip1559GasPool(final long blockNumber)`
- `long legacyGasPool(final long blockNumber)`

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* spotlessApply

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* TODO Add CLI command line flag `--Xeip1559-enabled`.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* update known hash

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* clean up

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* clean up

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* remove useless constructor

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* add spdx header

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
blockchain-trainer pushed a commit to blockchain-trainer/besu that referenced this pull request Apr 6, 2020
@AbdelStark @blockchain-trainer
[EIP-1559] Step 3 - Implement EIP-1559 logic in a manager class (hype…
7f67551 
…rledger#622)

* Added changelog entries for PR:
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* ### Description
BlockHeader object needs to be modified in order to add the new field (baseFee) as specified in the EIP.
We should take care about the RLP encoding/decoding of this structure since it has to include or not the new fields depending on whether we are pre fork or post fork.

- Update `core.BlockHeader.java`
    - Add `baseFee`
    - Update `readFrom` method for RLP decoding
    - Update `writeTo` method for RLP encoding
- Update `plugin.data.BlockHeader.java`
    - Add `getBaseFee` method

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* ### Description
Transaction object needs to be modified in order to add the 2 new fields (`gasPremium` and `feeCap`) as specified in the EIP.
We should take care about the RLP encoding/decoding of this structure since it has to include or not the new fields depending on whether we are pre fork or post fork.

- Update core `Transaction` object
    - Add gasPremium and feeCap fields
    - Update readFrom method for RLP decoding
    - Update writeTo method for RLP encoding
- Update plugin `Transaction` interface
    - Add `getGasPremium` and `getFeeCap` methods

This EIP introduces gasPremium and feeCap fields in the transaction. They need to be included in the signing mechanism.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* We need a manager class to hold EIP-1559 core logic. This helper class will be then used by glue code. This separation make testing simpler.
Specifically the following methods:

- `long computeBaseFee(final long parentBaseFee, final long parentGasUsed)`
- `boolean isValidBaseFee(final long parentBaseFee, final long baseFee)`
- `long eip1559GasPool(final long blockNumber)`
- `long legacyGasPool(final long blockNumber)`

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* spotlessApply

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* TODO Add CLI command line flag `--Xeip1559-enabled`.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* update known hash

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* clean up

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* clean up

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* remove useless constructor

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* add spdx header

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
Signed-off-by: viraj <blockchaintrainer.in@gmail.com>
AbdelStark added a commit that referenced this pull request Apr 6, 2020
@AbdelStark
[EIP-1559] Step 4 - Prepare EIP-1559 genesis field. (#657)
c58cb7c 
* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Prepare EIP-1559 genesis field.
The fork name is not known so for the moment we let eip1559 as a placeholder to update when the actual fork name is known.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 9, 2020
@AbdelStark
[EIP-1559] Step 5 - Gas price and gas limit block header validation r…
8b1a06f 
…ules (#660)

* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Implement gas limit and gas price block header validation rules as per specified in EIP-1559

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* guard with feature flag

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* address pr comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* address pr comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 9, 2020
@AbdelStark
[EIP-1559] Step 6 - Enforce the per transaction gas limit (#670)
bb74a42 
* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Implement gas limit and gas price block header validation rules as per specified in EIP-1559

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* We should enforce the per transaction gas limit, as specified by the EIP, to 8,000,000 gas.
Add a `TransactionInvalidReason`
Update the `validate` method in `MainnetTransactionValidator` to enforce this check.
Update the validation process in `MainnetBlockBodyValidator` to enforce this check for all transactions within a block.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* fix unit test

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Removed blockNumber from TransactionValidator.validate

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 10, 2020
@AbdelStark
[EIP-1559] Step 7 - Check transaction format for send raw transaction (…
5a5f4eb 
…#694)

* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Check transaction format for send raw transaction

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* remove EthSendRawTransactionTest.blokchainQueries field

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 14, 2020
@AbdelStark
Remove addRule method from BlockHeaderValidator (#705)
5867a4b 
* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* remove addRule method from BlockHeaderValidator

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit that referenced this pull request Apr 16, 2020
@AbdelStark
[EIP-1559] Step 8 - Disable GasLimitRangeAndDeltaValidationRule and u…
4a8f999 
…pdate ProofOfWorkValidationRule (#722)

* Added changelog entries for PR:
- #430
- #440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* For post EIP-1559 blocks we must disable GasLimitRangeAndDeltaValidationRule.
We must also update ProofOfWorkValidationRule to include base fee field in the hash computation.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Rename logger

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Rename logger

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Fix base fee computation and add tests.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>

* Address PR comments

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 20, 2020
@AbdelStark
Added changelog entries for PR:
047bf18 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 23, 2020
@AbdelStark
Added changelog entries for PR:
30b1e2b 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 23, 2020
@AbdelStark
Added changelog entries for PR:
e9b5094 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 24, 2020
@AbdelStark
Added changelog entries for PR:
74bb72c 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 27, 2020
@AbdelStark
Added changelog entries for PR:
81b9e8a 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 28, 2020
@AbdelStark
Added changelog entries for PR:
5ccfe43 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 29, 2020
@AbdelStark
Added changelog entries for PR:
9ea6bc5 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
AbdelStark added a commit to AbdelStark/besu that referenced this pull request Apr 30, 2020
@AbdelStark
Added changelog entries for PR:
09115f3 
- hyperledger#430
- hyperledger#440

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RatanRSur RatanRSur RatanRSur approved these changes

Assignees

@AbdelStark AbdelStark

Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AbdelStark @RatanRSur @alexandratran