Mempool Perf fix #1752
Mempool Perf fix #1752
Conversation
WalkthroughThe overarching changes involve integrating a new Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (14)
- go/common/gethencoding/geth_encoding.go (3 hunks)
- go/enclave/components/batch_executor.go (5 hunks)
- go/enclave/components/rollup_compression.go (7 hunks)
- go/enclave/enclave.go (7 hunks)
- go/enclave/evm/chain_context.go (2 hunks)
- go/enclave/evm/ethchainadapter/eth_chainadapter.go (8 hunks)
- go/enclave/evm/evm_facade.go (4 hunks)
- go/enclave/l2chain/l2_chain.go (4 hunks)
- go/enclave/nodetype/sequencer.go (4 hunks)
- go/enclave/storage/enclavedb/batch.go (4 hunks)
- go/enclave/storage/init/edgelessdb/001_init.sql (1 hunks)
- go/enclave/storage/init/sqlite/001_init.sql (2 hunks)
- go/enclave/storage/interfaces.go (1 hunks)
- go/enclave/storage/storage.go (6 hunks)
Files skipped from review due to trivial changes (1)
- go/enclave/storage/init/edgelessdb/001_init.sql
Additional comments: 53
go/enclave/evm/chain_context.go (3)
- 19-21: The addition of
gethEncodingServiceto theObscuroChainContextstruct is consistent with the PR's objective to integrate a new encoding service.- 25-29: The
NewObscuroChainContextfunction has been correctly updated to accept and initialize the newgethEncodingServicefield.- 46-46: The
GetHeadermethod now usesgethEncodingServiceto create an Ethereum header for a batch. Ensure that theFetchBatchmethod fromstoragehandles errors correctly and that theCreateEthHeaderForBatchmethod fromgethEncodingServiceis implemented correctly and tested.go/enclave/storage/init/sqlite/001_init.sql (1)
- 60-70: The addition of the
converted_hashcolumn to thebatchtable is consistent with the PR's objective to enhance the handling of Ethereum headers and batch processing. Ensure that all database operations that interact with thebatchtable are updated to handle the newconverted_hashcolumn correctly.go/enclave/evm/ethchainadapter/eth_chainadapter.go (7)
- 26-26: The addition of
gethEncodingto theEthChainAdapterstruct aligns with the PR's objective to use a new encoding service across various components.- 33-38: The
NewEthChainAdapterfunction has been correctly updated to accept the newgethEncodingparameter and initialize the corresponding struct field.- 60-60: The
CurrentBlockmethod now usesgethEncodingto convert a batch header to an Ethereum header. Ensure that theCreateEthHeaderForBatchmethod fromgethEncodingis implemented correctly and tested.- 86-86: The
GetBlockmethod's signature has been updated to usegethcommon.Hashinstead ofcommon.Hash. Ensure that all calls to this method are updated accordingly and that the method's implementation correctly handles the new type.- 111-111: The
CreateEthBlockFromBatchmethod fromgethEncodingis used here. Verify that this method is correctly implemented and tested.- 121-121: The
StateAtmethod's signature has been updated to usegethcommon.Hash. Ensure that all calls to this method are updated accordingly and that the method's implementation correctly handles the new type.- 130-130: The
IngestNewBlockmethod now usesgethEncodingto convert a batch to an Ethereum block. Verify that theCreateEthBlockFromBatchmethod is correctly implemented and tested.go/enclave/storage/interfaces.go (2)
- 58-58: The addition of the
FetchConvertedHashmethod to theBatchResolverinterface is consistent with the PR's objective to handle converted hashes in the system.- 67-67: The
StoreBatchmethod signature has been correctly updated to include aconvertedHashparameter. Ensure that all implementations of this method are updated to handle the new parameter correctly.go/enclave/evm/evm_facade.go (5)
- 37-37: The
ExecuteTransactionsfunction now requires an additionalgethEncodingServiceparameter. Ensure that all calls to this function are updated to pass the new parameter.- 50-50: The
CreateEthHeaderForBatchfunction is called from thegethEncodingServiceinterface. Verify that this function is correctly implemented and tested.- 161-161: The
initParamsfunction's signature has been modified to include thegethEncodingServiceparameter. Ensure that all calls to this function are updated to pass the new parameter.- 177-177: The
CreateEthHeaderForBatchfunction is called from thegethEncodingServiceinterface within theExecuteObsCallfunction. Verify that this function is correctly implemented and tested.- 212-212: The
initParamsfunction has been updated to create a newObscuroChainContextwith thegethEncodingService. Ensure that theNewObscuroChainContextfunction is correctly implemented and tested.go/enclave/l2chain/l2_chain.go (4)
- 32-34: The
obscuroChainstruct has been updated to include thegethEncodingServicefield, which is consistent with the PR's objectives.- 44-58: The
NewChainfunction has been updated to accept and initialize thegethEncodingServicefield. Ensure that all calls to this function are updated to pass the new parameter.- 153-153: The
ObsCallAtBlockfunction now usesgethEncodingServiceto execute an Ethereum call. Verify that theExecuteObsCallfunction fromevmis correctly implemented and tested.- 197-199: The
GetChainStateAtTransactionfunction now usesgethEncodingServiceto create an Ethereum header for a batch. Verify that theCreateEthHeaderForBatchfunction is correctly implemented and tested.go/common/gethencoding/geth_encoding.go (5)
- 49-52: The
EncodingServiceinterface has been correctly defined with the methodsCreateEthHeaderForBatchandCreateEthBlockFromBatch.- 54-60: The
gethEncodingServiceImplstruct has been introduced with a cache for converted values and is correctly initialized in theNewGethEncodingServicefunction.- 62-80: The
NewGethEncodingServicefunction has been updated to include a context parameter and initialize a cache. Ensure that the cache is correctly configured and that the context is handled appropriately throughout the service's methods.- 277-346: The
CreateEthHeaderForBatchmethod has been updated to use a cache for storing and retrieving values. Ensure that the caching mechanism is correctly implemented and that the method correctly handles cache misses and errors.- 366-384: The
CreateEthBlockFromBatchmethod has been updated to create a local block and assign values to its fields. Ensure that the method correctly initializes all necessary fields of thetypes.Blockand that the conversion fromlocalBlocktotypes.Blockis safe and correct.go/enclave/nodetype/sequencer.go (3)
- 51-51: The addition of
gethEncodingto thesequencerstruct is consistent with the PR's objective to integrategethEncodingServiceacross various components.- 66-74: The
NewSequencerfunction signature has been correctly updated to include thegethEncodingServiceparameter. Ensure that all calls to this constructor are updated accordingly.Verification successful
The search with
rghas identified a call toNewSequencerin the filego/enclave/enclave.go. However, the output does not show the parameters passed to the function, which are necessary to verify if thegethEncodingServiceparameter is included in the call. To confirm this, we need to inspect the parameters of theNewSequencerfunction call ingo/enclave/enclave.go.
The extracted parameters from the
NewSequencerfunction call ingo/enclave/enclave.goincludegethEncodingService. This confirms that the call has been updated to include the newgethEncodingServiceparameter as required by the updated function signature.Based on the information gathered, the review comment has been verified, and the necessary updates to the
NewSequencerfunction calls have been made to include thegethEncodingServiceparameter.* 302-307: The `StoreExecutedBatch` method now uses `gethEncoding` to create an Ethereum header for the batch and stores it with the batch in the storage. This change aligns with the PR's objectives and appears to be correctly implemented.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for NewSequencer function calls to ensure they include the new parameter. ast-grep --lang go --pattern $'NewSequencer($$$)' | grep -v 'gethEncodingService'Length of output: 80
Script:
#!/bin/bash # Search for calls to NewSequencer and display the parameters to verify the inclusion of gethEncodingService. ast-grep --lang go --pattern $'NewSequencer($$$)'Length of output: 50
Script:
#!/bin/bash # Use ripgrep to search for the string "NewSequencer" to find any potential calls. rg 'NewSequencer'Length of output: 792
Script:
#!/bin/bash # Extract the parameters passed to the NewSequencer function call in go/enclave/enclave.go rg 'NewSequencer\(' go/enclave/enclave.go -A 10Length of output: 253
go/enclave/components/batch_executor.go (3)
- 39-39: The
gethEncodingServicefield has been added to thebatchExecutorstruct. This change is consistent with the PR's objectives.- 51-57: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [54-64]
The
NewBatchExecutorfunction signature has been updated to include thegethEncodingServiceparameter. This is a necessary change for the integration of the new service.
- 425-425: The
processTransactionsmethod has been updated to passexecutor.gethEncodingServiceto theevm.ExecuteTransactionsfunction. This change is in line with the PR's objectives to use the new encoding service.go/enclave/storage/enclavedb/batch.go (3)
- 28-28: The SQL query string
bInserthas been renamed tobatchInsert. This is a minor change and does not affect functionality.- 52-52: The
WriteBatchAndTransactionsfunction now includes aconvertedHashparameter. This change is consistent with the PR's objectives to store the converted hash in the database.- 550-563: A new function
FetchConvertedBatchHashhas been introduced to retrieve the converted batch hash from the database. This function is necessary for the new functionality introduced in the PR.go/enclave/components/rollup_compression.go (5)
- 54-54: The
gethEncodingServicefield has been added to theRollupCompressionstruct.- 63-69: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [66-76]
The
NewRollupCompressionfunction now initializes thegethEncodingServicefield.
- 430-438: The
executeAndSaveIncompleteBatchesmethod now usesgethEncodingServiceto create Ethereum headers for batches and store them. Ensure that theCreateEthHeaderForBatchmethod handles errors correctly and that theStoreBatchmethod is correctly updated to handle the newconvertedHashparameter.- 460-465: The
executeAndSaveIncompleteBatchesmethod also usesgethEncodingServicefor the genesis batch. Verify that theCreateEthHeaderForBatchandStoreBatchmethods are correctly implemented and handle errors appropriately.- 502-507: Again in the
executeAndSaveIncompleteBatchesmethod,gethEncodingServiceis used for computing the batch. Ensure that theCreateEthHeaderForBatchandStoreBatchmethods are correctly implemented and handle errors appropriately.go/enclave/storage/storage.go (4)
- 60-61: The
cachedSharedSecretfield remains unchanged.- 147-161: The
FetchConvertedHashmethod has been added to fetch the converted hash of a batch. Ensure that the method correctly retrieves the converted hash from the database and handles errors appropriately.- 239-249: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [243-258]
The
FetchSecretmethod now caches the shared secret. Verify that the caching mechanism works correctly and that the shared secret is properly encoded and decoded.
- 400-407: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [386-404]
The
StoreBatchmethod signature has been updated to include aconvertedHashparameter. Ensure that theWriteBatchAndTransactionsmethod is updated to handle this new parameter and that the method correctly stores the batch and its converted hash in the database.go/enclave/enclave.go (8)
- 185-185: The
NewBatchExecutorfunction now includesgethEncodingServiceas a parameter.This change is consistent with the integration of
gethEncodingServiceacross the system.
- 192-192: The
NewRollupCompressionfunction now includesgethEncodingServiceas a parameter.This change is consistent with the integration of
gethEncodingServiceacross the system.
- 196-196: The
NewEthChainAdapterfunction now includesgethEncodingServiceas a parameter.This change is consistent with the integration of
gethEncodingServiceacross the system.
- 211-211: The
nodetype.NewSequencerfunction now includesgethEncodingServiceas a parameter.This change is consistent with the integration of
gethEncodingServiceacross the system.
- 235-235: The
l2chain.NewChainfunction now includesgethEncodingServiceas a parameter.This change is consistent with the integration of
gethEncodingServiceacross the system.
- 270-270: The
gethEncodingServicefield is set in theenclaveImplstruct within theNewEnclavefunction.This change is consistent with the integration of
gethEncodingServiceacross the system.
- 547-550: The
SubmitBatchfunction now usesgethEncodingServiceto create an Ethereum header for the batch.This change is consistent with the integration of
gethEncodingServiceacross the system.
- 553-553: The
StoreBatchmethod in thestorageinterface now includes aconvertedHashparameter.This change is consistent with the addition of the
converted_hashcolumn in the database schema and the integration ofgethEncodingService.
| @@ -89,6 +89,7 @@ type enclaveImpl struct { | |||
|
|
|||
| dataEncryptionService crypto.DataEncryptionService | |||
| dataCompressionService compression.DataCompressionService | |||
| gethEncodingService gethencoding.EncodingService | |||
There was a problem hiding this comment.
The gethEncodingService field is added to the enclaveImpl struct without any comments or documentation.
Consider adding comments to explain the purpose and usage of gethEncodingService within the enclaveImpl struct for better maintainability and clarity for future developers.
| @@ -171,6 +172,7 @@ | |||
| obscuroKey := crypto.GetObscuroKey(logger) | |||
| rpcEncryptionManager := rpc.NewEncryptionManager(ecies.ImportECDSA(obscuroKey)) | |||
|
|
|||
| gethEncodingService := gethencoding.NewGethEncodingService(storage, logger) | |||
There was a problem hiding this comment.
The instantiation of gethEncodingService is done within the NewEnclave function without error handling.
- gethEncodingService := gethencoding.NewGethEncodingService(storage, logger)
+ gethEncodingService, err := gethencoding.NewGethEncodingService(storage, logger)
+ if err != nil {
+ logger.Crit("Failed to create gethEncodingService", log.ErrKey, err)
+ return nil
+ }Ensure that errors are handled appropriately when creating new instances of services.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| gethEncodingService := gethencoding.NewGethEncodingService(storage, logger) | |
| gethEncodingService, err := gethencoding.NewGethEncodingService(storage, logger) | |
| if err != nil { | |
| logger.Crit("Failed to create gethEncodingService", log.ErrKey, err) | |
| return nil | |
| } |
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sum
Files selected for processing (7)
- go/common/cache_util.go (1 hunks)
- go/common/gethencoding/geth_encoding.go (3 hunks)
- go/enclave/crosschain/block_message_extractor.go (2 hunks)
- go/enclave/crosschain/common.go (2 hunks)
- go/enclave/events/subscription_manager.go (4 hunks)
- go/enclave/storage/storage.go (11 hunks)
- integration/simulation/utils.go (2 hunks)
Files skipped from review due to trivial changes (1)
- go/enclave/crosschain/block_message_extractor.go
Files skipped from review as they are similar to previous changes (2)
- go/common/gethencoding/geth_encoding.go
- go/enclave/storage/storage.go
Additional comments: 7
go/common/cache_util.go (1)
- 11-23: The
GetCachedValuefunction retrieves a value from the cache or uses theonFailedcallback if the value is not present. The error handling and caching logic seem appropriate.integration/simulation/utils.go (2)
- 34-34: The
setupSimTestLogfunction has been modified to change the log level. This change will reduce the verbosity of logs, which could impact debugging. Ensure this change aligns with the desired logging policy.Verify that the reduced log verbosity is acceptable for simulation tests.
- 132-132: The
sleepRndBtwfunction now divides the sleep duration by 5. This change affects the timing behavior of simulations and could have downstream effects on test outcomes.Confirm that the change in sleep duration calculation is intended and does not negatively impact the simulation tests.
go/enclave/crosschain/common.go (2)
- 8-8: The new import of the
logpackage is consistent with the changes in the file.- 28-29: The
lazilyLogReceiptChecksumfunction's parameter type has been changed. Ensure that all calls to this function have been updated to pass a pointer tocommon.L1Blockinstead of a string.Verification successful
The search within
block_message_extractor.gohas provided several instances where theblockvariable is used. Notably, the function signatures forStoreCrossChainValueTransfers,StoreCrossChainMessages, andgetCrossChainMessagesindicate thatblockis a pointer tocommon.L1Block. This confirms that theblockvariable passed tolazilyLogReceiptChecksumis of the correct type as per the updated function signature.Based on this information, we can conclude that the call to
lazilyLogReceiptChecksuminblock_message_extractor.gois using the correct parameter type, which is a pointer tocommon.L1Block.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to lazilyLogReceiptChecksum to ensure they pass the correct parameter type. ast-grep --lang go --pattern $'lazilyLogReceiptChecksum($_, $_, $_)'Length of output: 176
Script:
#!/bin/bash # Extract the specific line where lazilyLogReceiptChecksum is called to verify the parameter type. rg --context 0 'lazilyLogReceiptChecksum\(.*\)' ./go/enclave/crosschain/block_message_extractor.goLength of output: 153
Script:
#!/bin/bash # Find the declaration or assignment of the 'block' variable to verify its type. ast-grep --lang go --pattern $'var block *common.L1Block = $_'Length of output: 63
Script:
#!/bin/bash # Search for any assignment to the 'block' variable or its usage that would indicate its type. rg 'block :=' ./go/enclave/crosschain/block_message_extractor.goLength of output: 66
Script:
#!/bin/bash # Search for any usage of the 'block' variable within the block_message_extractor.go file to infer its type. rg 'block' ./go/enclave/crosschain/block_message_extractor.goLength of output: 2213
go/enclave/events/subscription_manager.go (2)
- 158-158: The change in the logging statement within the
GetSubscribedLogsForBatchfunction uses structured logging instead of formatted strings, which is a good practice for consistency and readability.- 244-244: The changes to the logging statements in the
filterLogsfunction also switch to structured logging. This is a positive change for the reasons mentioned above.Also applies to: 248-248, 253-253, 258-258, 270-270
| func CacheValue[V any](cache *cache.Cache[V], logger gethlog.Logger, key any, v V) { | ||
| err := cache.Set(context.Background(), key, v) | ||
| if err != nil { | ||
| logger.Error("Could not store value in cache", log.ErrKey, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
The CacheValue function stores a value in the cache and logs an error if it fails. The error handling is correct, but it might be beneficial to consider whether the error should be returned to the caller instead of just logged.
Consider returning the error from CacheValue to allow the caller to handle it as needed.
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
go/enclave/vkhandler/vk_handler.go
Outdated
| type customRnd struct { | ||
| } | ||
|
|
||
| func (rnd *customRnd) Read(p []byte) (n int, err error) { | ||
| for i := range p { | ||
| p[i] = 1 | ||
| } | ||
| return len(p), nil | ||
| } |
There was a problem hiding this comment.
The customRnd type and its Read method have been introduced to replace the standard random number generator with a non-random one that fills the byte slice with 1s. This is a significant change in behavior and could have security implications, as it replaces secure random number generation with a predictable pattern. This should only be used for testing purposes and not in production code.
go/enclave/vkhandler/vk_handler.go
Outdated
| } | ||
|
|
||
| // used for checking the source of slowness | ||
| var nonRandomTestReader = &customRnd{} |
There was a problem hiding this comment.
The global variable nonRandomTestReader is used for checking the source of slowness. Using a global variable for this purpose is not thread-safe and could lead to unpredictable behavior if used in a concurrent context. Additionally, this should be clearly documented as for testing purposes only and should not be present in production code.
go/enclave/vkhandler/vk_handler.go
Outdated
| // Encrypt returns the payload encrypted with the viewingKey | ||
| func (m *VKHandler) Encrypt(bytes []byte) ([]byte, error) { | ||
| if len(bytes) == 0 { | ||
| bytes = placeholderResult | ||
| } | ||
|
|
||
| encryptedBytes, err := ecies.Encrypt(rand.Reader, m.publicViewingKey, bytes, nil, nil) | ||
| encryptedBytes, err := ecies.Encrypt(nonRandomTestReader, m.publicViewingKey, bytes, nil, nil) |
There was a problem hiding this comment.
The Encrypt method has been modified to use nonRandomTestReader instead of rand.Reader. This change should be carefully reviewed to ensure that it is only for testing purposes, as it would make the encryption deterministic and insecure. If this is for testing, it should be wrapped in a conditional compilation flag or a configuration option to prevent it from being used in a production environment.
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
tudor-malene
temporarily deployed
to
dev-testnet
GitHub Actions
Inactive
| @@ -82,7 +82,7 @@ func (m *blockMessageExtractor) StoreCrossChainMessages(block *common.L1Block, r | |||
| return nil | |||
| } | |||
|
|
|||
| lazilyLogReceiptChecksum(fmt.Sprintf("Processing block: %s receipts: %d", block.Hash(), len(receipts)), receipts, m.logger) | |||
| lazilyLogReceiptChecksum(block, receipts, m.logger) | |||
There was a problem hiding this comment.
avoid the sprintf when the log level does not need it.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- go/common/cache_util.go (1 hunks)
- go/common/gethencoding/geth_encoding.go (3 hunks)
- go/common/types.go (1 hunks)
- go/enclave/enclave.go (7 hunks)
- go/enclave/storage/storage.go (11 hunks)
Files skipped from review due to trivial changes (1)
- go/common/types.go
Files skipped from review as they are similar to previous changes (4)
- go/common/cache_util.go
- go/common/gethencoding/geth_encoding.go
- go/enclave/enclave.go
- go/enclave/storage/storage.go
| func NewGethEncodingService(storage storage.Storage, logger gethlog.Logger) EncodingService { | ||
| // todo (tudor) figure out the best values | ||
| ristrettoCache, err := ristretto.NewCache(&ristretto.Config{ | ||
| NumCounters: 1000, // number of keys to track frequency of 100. |
There was a problem hiding this comment.
minor: comment says 100 but value says 1000
| transactions: b.Transactions, | ||
| withdrawals: nil, | ||
| } | ||
| block := *(*types.Block)(unsafe.Pointer(&lb)) |
There was a problem hiding this comment.
Maybe worth a comment on this line to say something like "these operations force casting of the localBlock to the geth Block type. This was necessary because we can't access the header directly on geth Block"
which I think is what's going on?
There was a problem hiding this comment.
I literally have this comment on the localBlock type
There was a problem hiding this comment.
Oh yeah sorry, I guess maybe worth a // cast to Block here but up to you, it slowed down my scanning because it looked like such a scary line lol.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- go/common/gethencoding/geth_encoding.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- go/common/gethencoding/geth_encoding.go
Why this change is needed
We noticed a performance issue with the mempool.
After investigation, it turned out that we weren't aligning the hashes of the converted geth blocks, so the mempool was always in "reorg" mode.
This PR aligns these hashes.
What changes were made as part of this PR
fmt.sprintfPR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks