[Protobuf] Working prototype #85
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rpl-ffl
force-pushed
the
rapol/feature/protobuf
branch
from
dd122db
to
8850e6c
Compare
1 task
rpl-ffl
force-pushed
the
rapol/feature/protobuf
branch
5 times, most recently
from
74fb259
to
6c8167f
Compare
cabrador
requested changes
Sep 24, 2024
rpl-ffl
changed the base branch from
main
to
rapol/feature/flag-substate-decoder
rpl-ffl
force-pushed
the
rapol/feature/protobuf
branch
from
a44d970
to
3627d2a
Compare
rpl-ffl
force-pushed
the
rapol/feature/flag-substate-decoder
branch
from
e1c967b
to
7f435fb
Compare
Co-authored-by: cabrador <84449820+cabrador@users.noreply.github.com>
rpl-ffl
force-pushed
the
rapol/feature/protobuf
branch
from
3627d2a
to
e0d0de9
Compare
rpl-ffl
force-pushed
the
rapol/feature/protobuf
branch
from
462db90
to
bf429ad
Compare
Base automatically changed from
rapol/feature/flag-substate-decoder
to
main
September 26, 2024 08:34
cabrador
requested changes
Sep 28, 2024
db/substate_encoding.go
Outdated
Comment on lines
89
to
95
| // encodeSubstate defensively defaults to "default" if nil | ||
| func (db *substateDB) encodeSubstate(ss *substate.Substate, block uint64, tx int) ([]byte, error) { | ||
| if db.encoding == nil { | ||
| db.SetSubstateEncoding("default") | ||
| } | ||
| return db.encoding.encode(ss, block, tx) | ||
| } |
There was a problem hiding this comment.
First I think implementation details does not belong to a func comment.
Second, do you think we should have this condition? I think this belongs into the factory func.
There was a problem hiding this comment.
Defensive defaults now removed in favor of having SetSubstate("default") in all public constructor of substateDB.
79974a4
I understand the performance reason to do this, but this leaves direct instantiation of &substateDB{...} (in tests, for example) to panic if encoding is never set. Any advice on this?
comments on encode***( Co-authored-by: cabrador <84449820+cabrador@users.noreply.github.com>
cabrador
approved these changes
Oct 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, geth transactions were recorded using RLP. The record-replay has switched to Protobuf instead, with 18M block onwards having only the Protobuf recording.
This change would allow substate to read Protobuf messages from disk along side RLP.
Todo: Create a decode function that takes a flag to select RLP/Protobuf
Note: Protobuf encode function is not implemented, but should be straight forward.
Type of change