Default receipt compaction to be enabled and update latest db version…

[jframe]

PR description

Enable receipt compaction by default. This also updates the database version to use receipt compaction for all new databases simplifying database version logic. Whether receipt compaction is enabled or not still depends on the --receipt-compaction-enabled flag.

Performance testing

No noticeable difference in engine API times over 7 days

Fixed Issue(s)

fixes #7157

Thanks for sending a pull request! Have you done the following?

• Checked out our contribution guidelines?
• Considered documentation and added the doc-change-required label to this PR if updates are required.
• Considered the changelog and included an update if required.
• For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

• unit tests: ./gradlew build
• acceptance tests: ./gradlew acceptanceTest
• integration tests: ./gradlew integrationTest
• reference tests: ./gradlew ethereum:referenceTests:referenceTests

[siladu]

This also updates the database version to use receipt compaction for all new databases simplifying database version logic.

Just to make check my understanding, does this mean that the following scenarios hold?

Scenario A

1. Before this PR, I have an existing database on with DATABASE_METADATA.json on version 2, e.g. am on Besu version 24.5.1 with default options, i.e. receipt compaction disabled.
2. I upgrade Besu to a release containing this PR
3. My DATABASE_METADATA.json is upgraded to version 3

Scenario B

1. Before this PR, I have an existing database with receipt-compaction explicitly enabled so DATABASE_METADATA.json is version 3
2. I upgrade Besu to a release containing this PR
3. My DATABASE_METADATA.json remains on version 3

Scenario C

1. Using a version of Besu containing this PR, I start a new database with receipt-compaction-enabled=false
2. My DATABASE_METADATA.json is still set to version 3

Scenario D
Downgrading to 24.5.1 after any of Scenario A-C results in DATABASE_METADATA.json remaining on version 3.

Scenario E
Downgrading beyond 24.5.1 after any of Scenario A-D would break compatibility.

[siladu]

Not sure if this is related to the change of default directly, but is it intended that you have to give an explicit "true" value for the option to work?

$besu --receipt-compaction-enabled
Missing required parameter for option '--receipt-compaction-enabled' (<receiptCompactionEnabled>)

Think I had a similar issue when I enabled bonsai-limit-trie-logs-enabled by default and had to use a fallbackValue: 7c21eed#diff-694f097ee7fb3140c34f4aed55d05b024c8faf509b09e71643d16ed68e1db67bR90

[jframe]

This also updates the database version to use receipt compaction for all new databases simplifying database version logic.

Just to make check my understanding, does this mean that the following scenarios hold?

Scenario A

1. Before this PR, I have an existing database on with DATABASE_METADATA.json on version 2, e.g. am on Besu version 24.5.1 with default options, i.e. receipt compaction disabled.
2. I upgrade Besu to a release containing this PR
3. My DATABASE_METADATA.json is upgraded to version 3

Yes

Scenario B

1. Before this PR, I have an existing database with receipt-compaction explicitly enabled so DATABASE_METADATA.json is version 3
2. I upgrade Besu to a release containing this PR
3. My DATABASE_METADATA.json remains on version 3

Yes

Scenario C

1. Using a version of Besu containing this PR, I start a new database with receipt-compaction-enabled=false
2. My DATABASE_METADATA.json is still set to version 3

Yes

Scenario D Downgrading to 24.5.1 after any of Scenario A-C results in DATABASE_METADATA.json remaining on version 3.

Yes it will remain on version 3, but if receipt compaction isn't enabled then a warning is printed in case they wanted to remove the compacted receipts
Database contains compacted receipts but receipt compaction is not enabled, new receipts will be not stored in the compacted format. If you want to remove compacted receipts from the database it is necessary to resync Besu. Besu can support both compacted and non-compacted receipts.

Scenario E Downgrading beyond 24.5.1 after any of Scenario A-D would break compatibility.

Yes in this Besu won't start and there will be an error saying the database is incompatible

[jframe]

Not sure if this is related to the change of default directly, but is it intended that you have to give an explicit "true" value for the option to work?

$besu --receipt-compaction-enabled
Missing required parameter for option '--receipt-compaction-enabled' (<receiptCompactionEnabled>)

Think I had a similar issue when I enabled bonsai-limit-trie-logs-enabled by default and had to use a fallbackValue: 7c21eed#diff-694f097ee7fb3140c34f4aed55d05b024c8faf509b09e71643d16ed68e1db67bR90

I broke this in the original PR accidentally by adding arity=1 option. That was added so it would work with a default false value, fallback might have worked in that case. Now it defaults to true don't need either trick.

Updated the PR so works with an implicit true value.

[siladu]

Think I had a similar issue when I enabled bonsai-limit-trie-logs-enabled by default and had to use a fallbackValue: 7c21eed#diff-694f097ee7fb3140c34f4aed55d05b024c8faf509b09e71643d16ed68e1db67bR90

I broke this in the original PR accidentally by adding arity=1 option. That was added so it would work with a default false value, fallback might have worked in that case. Now it defaults to true don't need either trick.

Updated the PR so works with an implicit true value.

Now, this silently sets receipt-compaction-enabled=false...

besu --receipt-compaction-enabled
...
2024-08-16 20:33:05.055+10:00 | main | WARN  | BesuControllerBuilder | Receipt compacting enabled? false

That's why I had to add fallbackValue="true" for limit-trie-logs.

Also, I had to add my own log to test this - reckon it's worth adding this to the config overview table?

[fab-10]

LGTM, just waiting for resolve the comments on the options definition

[fab-10]

you could also set arity = "0..1" so it works with or without the boolean parameter

[siladu]

In the docs of the version we're using: https://javadoc.io/doc/info.picocli/picocli/4.7.5/picocli/CommandLine.Option.html#arity--

By default, flags (boolean options) have arity "0..1",

Not sure if this once used to work, but the behaviour I see is that a valueless option now acts as a toggle. So if we enable it by default, specifying --receipt-compaction-enabled toggles and therefore disables the feature 😓

https://picocli.info/#_toggle_boolean_flags

When a flag option is specified on the command line picocli will set its value to the opposite of its default value.

Specifying fallBackValue="true" was the only way I could get this to work as intended.

[jframe]

I've changed to use fallBackValue="true". I forgot to push the updated changes on Friday. I think it's working now.

[jframe]

Think I had a similar issue when I enabled bonsai-limit-trie-logs-enabled by default and had to use a fallbackValue: 7c21eed#diff-694f097ee7fb3140c34f4aed55d05b024c8faf509b09e71643d16ed68e1db67bR90

I broke this in the original PR accidentally by adding arity=1 option. That was added so it would work with a default false value, fallback might have worked in that case. Now it defaults to true don't need either trick.
Updated the PR so works with an implicit true value.

Now, this silently sets receipt-compaction-enabled=false...

besu --receipt-compaction-enabled
...
2024-08-16 20:33:05.055+10:00 | main | WARN  | BesuControllerBuilder | Receipt compacting enabled? false

That's why I had to add fallbackValue="true" for limit-trie-logs.

Also, I had to add my own log to test this - reckon it's worth adding this to the config overview table?
I don't think it's worth it as I doubt it will probably be changed much in practice. I've also added a test for the implicit case so I know that's working.

[siladu]

LGTM

reckon it's worth adding this to the config overview table?

Probably only useful in the short term when users who set it are wanting to remove it once its default. Maybe we should just print a less readable but catch-all "effective config" line for times like this.