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:
main
package as plugin entrypointflag
package, for plugeth compatibilityNode()
lookup, so iterator parent path won't workWithIntermediateStateNodes
from builder func namesReadStorageReceipts
WriteAncientBlock
Changes 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:
libsecp256k1
C 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/secp256k1
even without the geth dependency.wrapmain
) which just imports and runs the plugethcmd
package.These (or at least 2) won't be needed once we refactor completely to plugeth types.
Next up:
Start
the service when the plugin loads, but shutdown won't be handled cleanly. Should be easy enough to add alongside API registration.--log.vmodule
flag for selective verbosity is not working for plugins, it's nice to have for debugging so we should fix it5214213cd6
to60472965df
Looks 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 objects
are stored in this table, where `key` is the blockstore-prefixed multihash key for the IPLD object
nitpick: in v5
key
is 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 events
rpcSub := 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 events
rpcSub := 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.Block
GetReceiptsByHash(hash common.Hash) types.Receipts
GetTd(hash common.Hash, number uint64) *big.Int
// TODO LockTrie is never used
We 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.StateAccount
accountRLP := make([]byte, len(it.LeafBlob()))
copy(accountRLP, it.LeafBlob())
Nice, removing these unneccesary/redundant allocations and
copy
s 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 receipts
What'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
969f9c3b14
to52debaa758
52debaa758
to9b937d9071
2d6cb835e2
to76b8359bea
b02435eee9
to0b9637b4e9
16c7b262c5
todee59f4e76
Hey @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
cerccicd
account.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.
08b9a9783a
to413d6afd38
4cc5eef23e
toedde732a1c
edde732a1c
to03f5622bea
The 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.