Skip to content

New Release#361

Merged
emmanuelm41 merged 2 commits into
mainfrom
dev
May 7, 2025
Merged

New Release#361
emmanuelm41 merged 2 commits into
mainfrom
dev

Conversation

@emmanuelm41
Copy link
Copy Markdown
Member

No description provided.

ziscky and others added 2 commits May 7, 2025 14:42
* fix: get miner from AwardBlockReward correctly

* fix: lint
* fix: avoid storing actor code on cache for undefined cids

* fix: consolidate f2 robust address into f4 for evm actors

* fix: typo
@emmanuelm41 emmanuelm41 requested a review from ziscky May 7, 2025 17:42
Copy link
Copy Markdown

@windsurf-bot windsurf-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other comments: 0 Blocking, 0 Optional, 3 Nit, 2 FYI
  • Nit: actors/v2/tools.go (126-130) Nit: Mamma mia! 🤌 This else block after the return statement is like putting pineapple on pizza - unnecessary! 🍕
    	if len(blockCids) == 0 {
    		return blockCid, errors.New("could not find block hash for message cid. Slice is empty")
    	}
    	blockCid = blockCids[0].Cid
    
  • Nit: actors/v2/tools.go (99-99) Nit: That's a spicy typo! 🍝 'paramater' should be 'parameter' in this error message.
    		logger.Errorf("Could not get parameter 'Params' inside tx '%s'", txType)
    
  • Nit: parser/v2/parser.go (374-375) Nit: Mamma mia! 🤌 You removed the newline after line 374 which makes the code look like a tangled plate of spaghetti! Let's keep our code as organized as a perfect lasagna layer.
    	txFrom, txTo := p.getFromToRobustAddresses(trace.Msg.From, trace.Msg.To)
    
    	return &types.Transaction{
    
  • FYI: actors/v2/miner/miner.go (173-174) FYI: Mamma mia! 🍝 Good cleanup removing that debug print statement! It's always nice to remove these little debugging breadcrumbs when they're no longer needed - keeps the code as clean as fresh pasta! 🤌
  • FYI: actors/cache/impl/zcache.go (301-302) FYI: Mamma mia! 🤌 Good catch on renaming the parameter from 'cid' to 'cidStr' to avoid confusion with the imported package. And adding that check for undefined CIDs? *chef's kiss* 🍝 That's-a smart defensive programming! Prevents storing undefined CIDs that would just create confusion later, like overcooked spaghetti! 🍝

Comment thread actors/v2/init/init.go
Comment thread actors/address.go
Comment on lines +17 to +23
if err == nil {
shortAddress, _ := address.NewFromString(shortAddressStr)
addrStr, err := actorCache.GetRobustAddress(shortAddress)
if err == nil {
addr, _ = address.NewFromString(addrStr)
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mamma mia! 🤌 You're ignoring errors from address.NewFromString() on lines 18 and 21. This is like making spaghetti without checking if the water is boiling! Consider handling these errors properly or at least logging them to avoid unexpected behavior.

Suggested change
if err == nil {
shortAddress, _ := address.NewFromString(shortAddressStr)
addrStr, err := actorCache.GetRobustAddress(shortAddress)
if err == nil {
addr, _ = address.NewFromString(addrStr)
}
}
shortAddress, err := address.NewFromString(shortAddressStr)
if err != nil {
logger.Warnf("Error converting short address %s: %v", shortAddressStr, err)
} else {
addrStr, err := actorCache.GetRobustAddress(shortAddress)
if err == nil {
addr, err = address.NewFromString(addrStr)
if err != nil {
logger.Warnf("Error converting robust address %s: %v", addrStr, err)
}
}
}

switch p := params.(type) {
case *legacyv1.AwardBlockRewardParams:
return p.Miner.String()
// Duplicate cases , they will cause a compile time error.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a spicy meatball! 🍕 The comment says these cases would cause compile-time errors, but it doesn't explain why. If these types have the same structure but would cause errors due to Go's type system, consider adding a more detailed comment explaining the situation. Otherwise, future developers might try to uncomment these lines and get confused. Clarity is the secret ingredient in good code, just like in a perfect marinara! 🍝

@emmanuelm41 emmanuelm41 merged commit 3c95b5e into main May 7, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants