Skip to content

x/axelarcork/keeper, x/cork/keeper: HandleRemoveManagedCellarsProposal should bail out immediately found=true to avoid needless/wasted iterations #299

@odeke-em

Description

@odeke-em

Summary of Bug

The loops here

for _, inputID := range p.CellarIds.Ids {
if existingID == common.HexToAddress(inputID) {
found = true
}
}
if !found {
outputCellarIDs.Ids = append(outputCellarIDs.Ids, existingID.String())
}

and

for _, proposedCellarID := range p.CellarIds.Ids {
proposedCellarAddress := common.HexToAddress(proposedCellarID)
found := false
for _, id := range cellarAddresses {
if id == proposedCellarAddress {
found = true
}

iterate looking for a specific target but once it has found it just keeps iterating and comparing over all all the cellar ids and at no point do we modify found=false in the loop. If we notice down below, we only append to the slice if !found hence we really should just end that loop immediately to avoid needless CPU cycles being wasted

Suggestion

 for _, inputID := range p.CellarIds.Ids { 
 	if existingID == common.HexToAddress(inputID) { 
 		found = true // We found the existing inputID
                 break
 	} 
 } 
  
 if !found { 
 	outputCellarIDs.Ids = append(outputCellarIDs.Ids, existingID.String()) 
 } 

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions