Initial statediff plugin #2
No reviewers
Labels
No Label
Copied from Github
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cerc-io/plugeth-statediff#2
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "wip"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Minimum viable implementation of statediff plugin.
Running:
Summary of changes:
mainpackage as plugin entrypointflagpackage, for plugeth compatibilityNode()lookup, so iterator parent path won't workWithIntermediateStateNodesfrom builder func namesReadStorageReceiptsWriteAncientBlockChanges we definitely need to upstream to
plugeth-utils(see dev branch):MarshalBinary)In order to support having (plu)geth itself as a dependency, I had to implement some workarounds in plugeth/plugeth-utils:
libsecp256k1C library is present in plugeth-utils as well as geth, causing CGO to find duplicate symbols when linking, so I modified it to just link against geth's definitions instead of building when passed the build taglinkgeth.plugeth-utils/restricted/crypto/secp256k1even without the geth dependency.wrapmain) which just imports and runs the plugethcmdpackage.These (or at least 2) won't be needed once we refactor completely to plugeth types.
Next up:
Startthe service when the plugin loads, but shutdown won't be handled cleanly. Should be easy enough to add alongside API registration.--log.vmoduleflag for selective verbosity is not working for plugins, it's nice to have for debugging so we should fix it5214213cd6to60472965dfLooks great! A few comments but nothing major. Glad to see the overall structure of the code hasn't changed much despite the size of the diff.
@ -258,0 +251,4 @@Our Postgres schemas are built around a single IPFS backing Postgres IPLD blockstore table(`ipld.blocks`) that conforms with[go-ds-sql](https://github.com/ipfs/go-ds-sql/blob/master/postgres/postgres.go). All IPLD objectsare stored in this table, where `key` is the blockstore-prefixed multihash key for the IPLD objectnitpick: in v5
keyis a full CID, not a blockstore-prefixed multihash@ -30,3 +29,4 @@const APIName = "statediff"// APIVersion is the version of the state diffing service API// TODO: match package version?Not sure this makes sense since the package can be updated and version changed without affecting the API
@ -57,2 +49,2 @@// create subscription and start waiting for eventsrpcSub := notifier.CreateSubscription()// Stream subscribes to statediff payloads as they are created.func (api *PublicAPI) Stream(ctx context.Context, params Params) (<-chan Payload, error) {Nice that they did away with the
*rpc.Subscription, returning a channel makes it much simpler@ -113,2 +91,2 @@// create subscription and start waiting for eventsrpcSub := notifier.CreateSubscription()// StreamCodeAndCodeHash writes all of the codehash=>code pairs at a given block to a websocket channel.func (api *PublicAPI) StreamCodeAndCodeHash(ctx context.Context, blockNumber uint64) (<-chan types.CodeAndCodeHash, error) {We haven't ever actually used this endpoint, should consider getting rid of it.
In that case, I vote for dropping it for now, pending a use case
That sounds good to me.
@ -0,0 +25,4 @@GetBlockByNumber(number uint64) *types.BlockGetReceiptsByHash(hash common.Hash) types.ReceiptsGetTd(hash common.Hash, number uint64) *big.Int// TODO LockTrie is never usedWe had this previously because the statediffing process would fall/lag far enough behind head, below the pruning threshold of a full node, such that the full node would sometimes prune away the state before the statediffing service had used it. This shouldn't be a problem anymore after the multitude of improvements made to performance.
@ -0,0 +50,4 @@for event := range bufferChan {block := utils.MustDecode[types.Block](event.Block)// TODO: apparently we ignore the logs// logs := utils.MustDecode[types.Log](chainEvent.Logs)We should get the logs when we lookup the Receipts and pack them into the statediff payload.
@ -286,3 +266,2 @@func (sdb *StateDiffBuilder) processStateValueNode(it trie.NodeIterator, parentBlob []byte) (*sdtypes.AccountWrapper, error) {var account types.StateAccountaccountRLP := make([]byte, len(it.LeafBlob()))copy(accountRLP, it.LeafBlob())Nice, removing these unneccesary/redundant allocations and
copys could have a noticeable impact on performance considering the number of times these operations are performed per block/statediff@ -81,6 +81,31 @@ func processReceiptsAndLogs(rcts []*types.Receipt) ([]*EthReceipt, [][]*EthLog,return ethRctNodes, ethLogNodes, nil}// // processReceiptsAndLogs will take in receiptsWhat's this commented out stuff for?
for disposal, just left over from checking something
@ -0,0 +25,4 @@func Initialize(ctx core.Context, pl core.PluginLoader, logger core.Logger) {log.SetDefaultLogger(logger)// lvl, err := strconv.ParseInt(ctx.String("verbosity"), 10, 8)Is this from geth upstream or here for future use? If not, lets remove the commented out stuff.
no, just left over from debugging logging
969f9c3b14to52debaa75852debaa758to9b937d90712d6cb835e2to76b8359beab02435eee9to0b9637b4e916c7b262c5todee59f4e76Hey @roysc are we waiting for the integration testing to be complete before merging this?
Right, and the tests are waiting on a way to provision access tokens on the runners so that image builds can access the Gitea server (@telackey is currently working on a solution).
I thought that problem was resolved?
(see: https://github.com/cerc-io/hosting/issues/49)
That lets us use a token to pull packages into the image, but it was my understanding that there is still no token available to use on the runner (?)
Might have been a miscommunication, as I wasn't attempting to create the token for use in CI as well (though I did test manually with a token created on my own account), since I do not have admin access to the organization or access to the
cerccicdaccount.That said, I am happy to do it. I just need the proper access.
I made the token,
${{ secrets.CICD_REPO_TOKEN }}, and tweaked the plugeth and plugeth-statediff Dockerfiles / workflows to make use of it. More details are here: https://github.com/cerc-io/hosting/issues/49While the actions still have other failures, they are getting past the Docker build.
08b9a9783ato413d6afd384cc5eef23etoedde732a1cedde732a1cto03f5622beaThe CI build now reaches the actual testing step (unit tests still have some edge cases that were already failing on the geth fork), so I will go ahead and merge this.