Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(x/slashing/keeper): hoist non-changing addresses parsing out of redelegation loop #18035

Merged

Conversation

odeke-em
Copy link
Collaborator

This change hoists out, the parsing of non-changing validator address and delegator address which before were being re-parsed in every loop of the redelegation entries, parsing them once and reusing the already parsed addresses.

Fixes #18032

@odeke-em odeke-em requested a review from a team as a code owner October 10, 2023 08:40
@github-actions

This comment has been minimized.

@julienrbrt julienrbrt added the backport/v0.50.x PR scheduled for inclusion in the v0.50's next stable release label Oct 10, 2023
…edelegation loop

This change hoists out, the parsing of non-changing validator address and
delegator address which before were being re-parsed in every loop of
the redelegation entries, parsing them once and reusing the already
parsed addresses.

Fixes #18032
@odeke-em odeke-em force-pushed the x-slashing-hoist-validator+delegator-address-parsing branch from 2db64a3 to b78c272 Compare October 10, 2023 08:43
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

lgtm!

@@ -287,6 +287,16 @@ func (k Keeper) SlashRedelegation(ctx context.Context, srcValidator types.Valida
totalSlashAmount = math.ZeroInt()
bondedBurnedAmount, notBondedBurnedAmount := math.ZeroInt(), math.ZeroInt()

valDstAddr, err := k.validatorAddressCodec.StringToBytes(redelegation.ValidatorDstAddress)
if err != nil {
return math.ZeroInt(), fmt.Errorf("SlashRedelegation: could not parse validator destination address: %w", err)
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for adding context to the errors 🙏🏾

@tac0turtle tac0turtle added this pull request to the merge queue Oct 10, 2023
Merged via the queue into main with commit 7d7d490 Oct 10, 2023
49 of 50 checks passed
@tac0turtle tac0turtle deleted the x-slashing-hoist-validator+delegator-address-parsing branch October 10, 2023 10:16
mergify bot pushed a commit that referenced this pull request Oct 10, 2023
…edelegation loop (#18035)

(cherry picked from commit 7d7d490)

# Conflicts:
#	x/staking/keeper/slash.go
tac0turtle added a commit that referenced this pull request Oct 10, 2023
…edelegation loop (backport #18035) (#18037)

Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
Co-authored-by: marbar3778 <marbar3778@yahoo.com>
@faddat faddat mentioned this pull request Nov 8, 2024
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v0.50.x PR scheduled for inclusion in the v0.50's next stable release C:x/staking
Projects
None yet
3 participants