feat: height-based tx parsing#282
Merged
Merged
Conversation
emmanuelm41
approved these changes
Feb 3, 2025
Member
emmanuelm41
left a comment
There was a problem hiding this comment.
Just a few comments, but it looks amazing
Member
|
@ziscky we need to fix CI first |
Height Based Parsing 3
Height Based Parsing 2
Closed
lucaslopezf
approved these changes
Feb 25, 2025
Contributor
lucaslopezf
left a comment
There was a problem hiding this comment.
I really like the strategy we took! And the fact that everything is tested!! Very good PR.
I'm approving it because I don't see anything blocking. I left some comments that could be addressed in a second iteration (if applicable).
- Personally, I find maps more convenient than switches, but that's just a matter of preference (I said this because we're using switch a lot, in some place I think we can use maps and in other places we can use a struct, I put an example below)
- I like the defensive strategy! And I imagine we did a lot of things for backward compatibility reasons, but since we're at it, in the future:
- If you like the idea, instead of using switch cases, we could use a struct and iterate over it (I left an example in the miner package).
- We could use Golem's log instead of Zap.
- Add ctx parameter to principal interface
| @@ -0,0 +1,495 @@ | |||
| package miner | |||
Contributor
There was a problem hiding this comment.
I put here as an example and idea, you can discard if don't like it. But maybe instead of switch we can do something like this in every package (of course in a second iteration)
// VersionParser groups the configuration for parsing according to a specific version.
type VersionParser struct {
// IsSupported is a function that checks whether the version is compatible with the requested network and height.
IsSupported func(network string, height int64) bool
// Template contains an instance (usually a struct) that will be used to parse the message or return value.
Template interface{}
// FieldKey indicates whether parser.ParamsKey or parser.ReturnKey should be used.
FieldKey string
}
// Definition of supported versions for GetAvailableBalanceExported.
// It is easy to add new versions or modify the priority order.
var availableBalanceParsers = []VersionParser{
{
IsSupported: tools.V24.IsSupported,
Template: &miner15.GetAvailableBalanceReturn{},
FieldKey: parser.ReturnKey,
},
{
IsSupported: tools.V23.IsSupported,
Template: &miner14.GetAvailableBalanceReturn{},
FieldKey: parser.ReturnKey,
},
{
IsSupported: tools.V22.IsSupported,
Template: &miner13.GetAvailableBalanceReturn{},
FieldKey: parser.ReturnKey,
},
{
IsSupported: tools.V21.IsSupported,
Template: &miner12.GetAvailableBalanceReturn{},
FieldKey: parser.ReturnKey,
},
{
// For V19 and V20 grouped together.
IsSupported: func(network string, height int64) bool {
return tools.AnyIsSupported(network, height, tools.V19, tools.V20)
},
Template: &miner11.GetAvailableBalanceReturn{},
FieldKey: parser.ReturnKey,
},
{
IsSupported: tools.V18.IsSupported,
Template: &miner10.GetAvailableBalanceReturn{},
FieldKey: parser.ReturnKey,
},
}
// GetAvailableBalanceExported uses the version configuration to select the correct parsing logic.
func (*Miner) GetAvailableBalanceExported(network string, height int64, rawReturn []byte) (map[string]interface{}, error) {
for _, vp := range availableBalanceParsers {
if vp.IsSupported(network, height) {
return parseGeneric(rawReturn, nil, false, vp.Template, vp.Template, vp.FieldKey)
}
}
// Handling for versions prior to V17 (for example)
if tools.AnyIsSupported(network, height, tools.VersionsBefore(tools.V17)...) {
return map[string]interface{}{}, fmt.Errorf("%w: %d", actors.ErrInvalidHeightForMethod, height)
}
return nil, fmt.Errorf("%w: %d", actors.ErrUnsupportedHeight, height)
}
Contributor
Author
There was a problem hiding this comment.
I really like this! Thanks
This file contains hidden or 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
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.
Actors V2
Overview
ActorsV2 is a new implementation of the actor parsing logic that is more flexible and easier to maintain while properly handling all legacy spec-actors and new builtin-actors structs. This ensures that the parser can properly parse all actor messages for any network version.
Design
The
ActorParserstruct is tasked with parsing actor messages and is composed of ahelper.Helperand azap.Logger.Each actor implements the
Actorinterface, which includes the following methods:Name() string: Returns the actor's name.Parse(network string, height int64, txType string, msg *parser.LotusMessage, msgRct *parser.LotusMessageReceipt, mainMsgCid cid.Cid, key filTypes.TipSetKey) (map[string]interface{}, *types.AddressInfo, error): Parses the actor message.TransactionTypes() map[string]any: Provides a map of all transaction types supported by the actor.Actor Folder Structure
Each actor is organized within its own directory under
actors/v2. The directory contains the following files:generic.go: Defines generic functions, typed with specific builtin-actors/spec-actors version structs, for parsing actor messages.parse.go: Contains the switch case logic for parsing specific transaction types for the actor.Testing
There are different types of tests to ensure comprehensive coverage:
Actor Support Verification
Test Function:
TestAllActorsSupportedThis test verifies that all actors in the latest builtin-actor release and all legacy spec-actors are supported by the parser.
It will fail if any actor is not supported.
To add a new actor:
actors/v2.Actorinterface.ActorParser.GetActorfunction.Method Coverage Verification
Test Function:
TestMethodCoverageThis test verifies that all methods exposed by the actor in all builtin-actor and spec-actor releases are supported by the actors.
It will fail if any method is not covered.
To add support for a new method:
TransactionTypesmap.Parsefunction.Network Version Coverage Verification
Test Function:
TestVersionCoverageThis test verifies that all the actor methods can correctly handle all network versions ( decided by the height of the block ).
It will fail if any network version is not supported.
To add support for a new network version:
tools/version_mapping.go.Actor Functionality Tests
Test Location:
actors/tests/{actor_name}_test.goThese tests, originally developed for actors version 1 (v1), are designed to validate the functionality of actors by comparing their function outputs against a set of pre-calculated expected values. These expected values are stored in the data/actors/{actor_name} directory.
Each actor undergoes both v1 and v2 tests, and passing both test suites is a mandatory requirement. This dual testing approach ensures backward compatibility and adherence to established specifications.
Important Note: The pre-computed data currently stored within data/actors corresponds to network version V20. If testing against a different network version is required, the
cmd/tracedltool provides a mechanism for automatically updating the stored data to the desired network version. This ensures that tests are always executed against the correct expected values for the target network version.These tests are designed to ensure that the actor parser accurately handles all releases of both builtin-actors and spec-actors. They are configured to automatically fail upon the release of any new builtin-actor version. This failure mechanism guides developers to the necessary modifications, thereby eliminating the need for manual verification of the parser with each new Filecoin upgrade.
Compatibility
The
ActorParseris designed to be backwards compatible and a drop in replacement for ActorsV1.Misc
Filecoin Network Version - Actor Version Mapping
The following table shows the mapping of Filecoin network versions to actor versions:
🔗 zboto Link