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

TrieLogFactory plugin support #5440

Merged
merged 30 commits into from
May 15, 2023
Merged

TrieLogFactory plugin support #5440

merged 30 commits into from
May 15, 2023

Conversation

garyschulte
Copy link
Contributor

@garyschulte garyschulte commented May 6, 2023 •
edited
Loading

PR description

PR does some plumbing to get TrieLogs and TrieLogFactory into the plugin-api, such that we can have a pluggable TrieLog serializer/deserializer and event observer. Having trielogs in plugin-api also facilitates trielog push and pull to remote services.

notable changes/rearrangement:

  • in plugin-api/services

    • adds TrieLogService
    • adds plugin-api/services/trielog package:
      • TrieLog, now plugin interface for TrieLogLayer
      • TrieLog.LogTuple, now plugin interface for BonsaiValue
      • TrieLogEvent
      • TrieLogObserver
      • TrieLogAccumulator, now plugin interface for BonsaiWorldStateUpdateAccumulator

  • in data types:

    • moves StorageSlotKey, into data-types
    • adds AccountValue, in data-types, now interface for StateTrieAccountValue and BonsaiAccount

  • writeRlp and writeInnerRlp moved from BonsaiValue into TrieLogFactory

  • TrieLog now exposes the account, code, and storage maps directly rather than only via stream

Fixed Issue(s)

fixes protocol-misc # 756

@github-actions
Copy link

github-actions bot commented May 6, 2023

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

@garyschulte garyschulte force-pushed the zkbesu-756-plugin-version branch 2 times, most recently from 521c1ca to 3587329 Compare May 9, 2023 22:52
@garyschulte garyschulte marked this pull request as ready for review May 10, 2023 15:14
@garyschulte garyschulte mentioned this pull request May 10, 2023
Merged
github-advanced-security[bot]
github-advanced-security bot found potential problems May 10, 2023
View reviewed changes
plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogProvider.java
/**
* Block and TrieLog layer composition, used for returning a range of TrieLog layers.
*
* @param blockNumber the block number

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags

@param tag "blockNumber" does not match any actual type parameter of type "TrieLogRangeTuple".
plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogProvider.java
* Block and TrieLog layer composition, used for returning a range of TrieLog layers.
*
* @param blockNumber the block number
* @param trieLog the associated TrieLog layer

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags

@param tag "trieLog" does not match any actual type parameter of type "TrieLogRangeTuple".
garyschulte added a commit to Consensys/linea-besu that referenced this pull request May 10, 2023
@garyschulte
dockerfile and github token hackery to build image (build fails for l…
668948f 
…inea until merge of hyperledger#5440)

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte mentioned this pull request May 10, 2023
Closed
github-advanced-security[bot]
github-advanced-security bot found potential problems May 12, 2023
View reviewed changes
plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogProvider.java
/**
* Block and TrieLog layer composition, used for returning a range of TrieLog layers.
*
* @param blockHash the block hash

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags

@param tag "blockHash" does not match any actual type parameter of type "TrieLogRangeTuple".
Copy link
Contributor

Choose a reason for hiding this comment

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

is this alert because it's a record? javadoc not 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.

Yeah, it is not clear what the linter wants here.

macfarla
macfarla reviewed May 12, 2023
View reviewed changes
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

so many tiny changes! a few comments

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/trielog/TrieLogLayer.java Outdated Show resolved Hide resolved
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/trielog/TrieLogLayer.java Outdated
@@ -43,11 +43,13 @@
* <p>In this particular formulation only the "Leaves" are tracked" Future layers may track patrica
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* <p>In this particular formulation only the "Leaves" are tracked" Future layers may track patrica
* <p>In this particular formulation only the "Leaves" are tracked Future layers may track patricia

plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogProvider.java
/**
* Block and TrieLog layer composition, used for returning a range of TrieLog layers.
*
* @param blockHash the block hash
Copy link
Contributor

Choose a reason for hiding this comment

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

is this alert because it's a record? javadoc not needed?

@garyschulte garyschulte force-pushed the zkbesu-756-plugin-version branch 3 times, most recently from 71cf24e to bfe7876 Compare May 12, 2023 08:00
gfukushima
gfukushima approved these changes May 15, 2023
View reviewed changes
Copy link
Contributor

@gfukushima gfukushima left a comment

Choose a reason for hiding this comment

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

lgtm few minor questions

.../core/src/main/java/org/hyperledger/besu/ethereum/bonsai/trielog/AbstractTrieLogManager.java Outdated Show resolved Hide resolved
besu/src/main/java/org/hyperledger/besu/components/BesuPluginContextModule.java Outdated
import dagger.Provides;

/**
* A dagger module that know how to create the BesuCommand, which collects all configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

think this was copied over the BesuCommand module, if so could we adjust the description to keep docs coherent.

...core/src/main/java/org/hyperledger/besu/ethereum/bonsai/cache/CachedWorldStorageManager.java Outdated
}

Stream<Long> rangeAsStream(final long fromBlockNumber, final long toBlockNumber) {
if (Math.abs(toBlockNumber - fromBlockNumber) > 1000L) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this limit number coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there isn't a use for this feature other than the shomei plugin currently, 1k is a compromise between shomei batch sync sizes and a reasonable upper bound on an corresponding rpc payload size. The payload size can vary based on the chain's gas limit and operations performed during the range. Using linea as a guidepost, an rpc request for blocks 0-1k weighs in at 510kb, whereas blocks 600k->601k are ~60mb.

I will add a class level constant for this rather than a 'magic number' however. 👍

@garyschulte
ZkTrieLogFactoryImpl, rebased off of main
5b22397 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
code before account
d92bcfd 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
trielog shipping observer
4dc1832 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
use hex encoding in trielog shipping, add a TESTING default ZkTrieLog…
6b10603 
…Observer to AbstractTrieLogManager

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
fix unintended recursion
0ca5fc0 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
filter out self destruct storage changes in ZkTrieLogImpl
4eb6e1d 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
store both slotHash and slotKey, defer to shomei to handle it as it s…
1248885 
…ees fit

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
reorder trielog storage to differentiate between ZERO slot key and nu…
b6c4ed2 
…ll slot key

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
do not filter unchanged accounts for zktrielogfactory
08120b0 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
use blockHeader during trielog construction, add blockNumber to triel…
f089122 
…og, add blockHeader to trielogaddedevent

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
add blockNumber to rpc call to shomei
e1f8dc7 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
add isSyncing to the trieLogParameter and ZkTrieLogObserver
f873ae5 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
initial plumbing for trielog shipping plugin
c659c4a 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
halfway through generics hell
5e8c35f 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
TrieLogs in plugin data
126f46d 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
end of friday, green build with plugin refactoring, still need to add…
a032562 
… TrieLog Plugin Service

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
remove errant reference to ZkTrieLogFactory
0b37f10 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
adding dagger-wired plugincontext and TrieLogService
3cd90d2 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
javadoc and fixes
3c867b9 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
add plugin observer subscription
e7ad166 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
plugin-api hash
c5e2019 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
fix besuComponent ref, move addService up into BesuContext
d5724c2 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
add TrieLogRangePair composition of blocknumber and TrieLog, use that…
1b5f858 
… for getTrieLogByRange return

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
javadoc
d4e79c6 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
move TrieLog from datatypes to plugin-api
2762389 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
add blockHash to TrieLogRangeTuple
f479ea0 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
adjust besuComponent mocks
483346b 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
spotless and javadoc
67b02f2 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
javadoc
402caec 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte
review feedback
bae88e3 
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte enabled auto-merge (squash) May 15, 2023 17:37
@garyschulte garyschulte merged commit c85841d into hyperledger:main May 15, 2023
@garyschulte garyschulte deleted the zkbesu-756-plugin-version branch May 15, 2023 18:40
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
@garyschulte @elenduuche
TrieLogFactory plugin support (hyperledger#5440)
eb07773 
* TrieLogs in plugin data
* adds dagger-wired plugincontext and TrieLogService
* add getTrieLogByRange, TrieLogRangeTuple composition
---------

Signed-off-by: garyschulte <garyschulte@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
@garyschulte @eum602
TrieLogFactory plugin support (hyperledger#5440)
b515a09 
* TrieLogs in plugin data
* adds dagger-wired plugincontext and TrieLogService
* add getTrieLogByRange, TrieLogRangeTuple composition
---------

Signed-off-by: garyschulte <garyschulte@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macfarla macfarla macfarla left review comments

@gfukushima gfukushima gfukushima approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@garyschulte @macfarla @gfukushima