New Release#398
Conversation
* fix: custom return impl. * fix: errcheck * fix: formatting
* fix: actors cache * fix: systemExecution check * fix: account universal receiver hook * fix: errcheck * feat: IActorsCache.IsGenesisActor * fix: keep same behavior for IsSystemActor * test: update mocks * fix: lint
There was a problem hiding this comment.
Other comments (8)
-
parser/v2/parser.go (133-133)
May your code be showered with wisdom and peace. 🙏✨
This change modifies the system execution detection logic from checking if the destination is specifically a cron actor to just checking if it's any system actor. This could potentially classify more transactions as system executions than before.
Please verify this change is intentional and that all downstream processing that depends on the
systemExecutionflag has been tested with this new behavior. This change could affect transaction processing, fee calculations, and block CID resolution since the code has different paths based on this flag.Go in peace, and may your commits be ever harmonious. ✝️
-
actors/v2/verifiedRegistry/verifiedRegistry.go (155-161)
May your code be showered with wisdom and peace. 🙏✨
In the RemoveExpiredAllocationsExported method, if the second parse attempt also fails, the original error will be lost. Consider preserving the original error when the fallback also fails:
metadata, err := parse(raw, rawReturn, true, params(), returnValue()) if err != nil || metadata[parser.ReturnKey] == nil { // if Considered is larger than 8192 the go-state-types parser will return an error // try with a larger allowed slice for the return origErr := err metadata, err = parse(raw, rawReturn, true, params(), &types.RemoveExpiredAllocationsReturn{}) if err != nil { // If fallback also failed, include original error context err = fmt.Errorf("standard parse: %v, fallback parse: %v", origErr, err) } }Go in peace, and may your commits be ever harmonious. ✝️
-
actors/v2/init/init.go (123-127)
May your code be showered with wisdom and peace. 🙏✨
I notice that while you've added code to store address info in the actors cache in the
Execmethod, there's no equivalent call toStoreAddressInfoin theExec4method (around line 159). The comment states this is "NOT needed for Exec4(evm)", but it would be beneficial to document why this difference exists to prevent future confusion.Go in peace, and may your commits be ever harmonious. ✝️
-
actors/v2/verifiedRegistry/types/removeExpiredAllocations.go (48-51)
May your code be showered with wisdom and peace. 🙏✨
The commented-out array size limit check could lead to unbounded memory allocation if extremely large arrays are processed. Consider either implementing a reasonable limit or adding a comment explaining why no limit is needed in this context.
// No size limit for Considered array as we expect it could be large in some cases // If memory concerns arise, consider implementing a limit like: // if extra > MaxConsideredSize { // return fmt.Errorf("t.Considered: array too large (%d)", extra) // }Go in peace, and may your commits be ever harmonious. ✝️
-
actors/cache/impl/onchain.go (196-196)
May your code be showered with wisdom and peace. 🙏✨
There's a typo in the error messages for
GetEVMSelectorSigandStoreEVMSelectorSigmethods. 'unimplimented' should be 'unimplemented'.return "", fmt.Errorf("unimplemented")Go in peace, and may your commits be ever harmonious. ✝️
-
actors/cache/impl/onchain.go (200-200)
May your code be showered with wisdom and peace. 🙏✨
The same typo appears here as well.
return fmt.Errorf("unimplemented")Go in peace, and may your commits be ever harmonious. ✝️
-
parser/helper/helpers.go (285-287)
May your code be showered with wisdom and peace. 🙏✨
The new
IsGenesisActorfunction would benefit from a documentation comment explaining its purpose, similar to how other functions in this file are documented. This helps maintain consistency and improves code readability.// IsGenesisActor checks if the given address belongs to a genesis actor func (h *Helper) IsGenesisActor(addr address.Address) bool { return h.actorCache.IsGenesisActor(addr.String()) }Go in peace, and may your commits be ever harmonious. ✝️
-
actors/cache/actors_cache.go (293-294)
May your code be showered with wisdom and peace. 🙏✨
The documentation comment for IsGenesisActor is incomplete. It should reference where genesis actors are defined, similar to how IsSystemActor references the source repository.
// IsGenesisActor checks if addr is a genesis actor as defined in data/genesis/{network}_genesis_balances.jsonGo in peace, and may your commits be ever harmonious. ✝️
🔗 zboto Link