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.

Sign up for GitHub

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

storage/client: don't crash on 0 delegated shares #719

Merged
merged 1 commit into from
Jul 2, 2024
Merged

storage/client: don't crash on 0 delegated shares #719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changelog/719.bugfix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
storage/client: don't crash on delegations with 0 shares
69 changes: 59 additions & 10 deletions storage/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,10 +844,19 @@ func (c *StorageClient) Account(ctx context.Context, address staking.Address) (*
}

// Computes shares worth given total shares and total balance.
func amountFromShares(shares common.BigInt, totalShares common.BigInt, totalBalance common.BigInt) common.BigInt {
func amountFromShares(shares common.BigInt, totalShares common.BigInt, totalBalance common.BigInt) (common.BigInt, error) {
if shares.IsZero() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ℹ️ db has some leftover cases where shares=0 and totalShares=0. this prevents us from complaining in that case

(is that right? in my understanding from the PR description and related PRs)

Copy link
Member Author

Choose a reason for hiding this comment

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

is that right? in my understanding from the PR description and related PRs

Yes, although I'm not 100% sure if shares=0 and totalShares=0 will still exist after all PRs are deployed and we do a reindex.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will recheck in future

return common.NewBigInt(0), nil
}
if totalShares.IsZero() {
// Shouldn't happen unless there is an invalid DB state. Don't panic since this is exposed
// in a public API.
return common.NewBigInt(0), fmt.Errorf("total shares is zero")
}

amount := new(big.Int).Mul(&shares.Int, &totalBalance.Int)
amount.Quo(amount, &totalShares.Int)
return common.BigInt{Int: *amount}
return common.BigInt{Int: *amount}, nil
}

// Delegations returns a list of delegations.
Expand All @@ -874,15 +883,25 @@ func (c *StorageClient) Delegations(ctx context.Context, address staking.Address
Delegator: address.String(),
}
var shares, escrowBalanceActive, escrowTotalSharesActive common.BigInt
if err := res.rows.Scan(
if err = res.rows.Scan(
&d.Validator,
&shares,
&escrowBalanceActive,
&escrowTotalSharesActive,
); err != nil {
return nil, wrapError(err)
}
d.Amount = amountFromShares(shares, escrowTotalSharesActive, escrowBalanceActive)
d.Amount, err = amountFromShares(shares, escrowTotalSharesActive, escrowBalanceActive)
if err != nil {
c.logger.Error("failed to compute delegated amount from shares (inconsistent DB state?)",
"delegator", address.String(),
"delegatee", d.Validator,
"shares", shares,
"total_shares", escrowTotalSharesActive,
"total_balance", escrowBalanceActive,
"err", err,
)
}
d.Shares = shares

ds.Delegations = append(ds.Delegations, d)
Expand Down Expand Up @@ -915,15 +934,25 @@ func (c *StorageClient) DelegationsTo(ctx context.Context, address staking.Addre
Validator: address.String(),
}
var shares, escrowBalanceActive, escrowTotalSharesActive common.BigInt
if err := res.rows.Scan(
if err = res.rows.Scan(
&d.Delegator,
&shares,
&escrowBalanceActive,
&escrowTotalSharesActive,
); err != nil {
return nil, wrapError(err)
}
d.Amount = amountFromShares(shares, escrowTotalSharesActive, escrowBalanceActive)
d.Amount, err = amountFromShares(shares, escrowTotalSharesActive, escrowBalanceActive)
if err != nil {
c.logger.Error("failed to compute delegated amount from shares (inconsistent DB state?)",
"delegator", address.String(),
"delegatee", d.Validator,
"shares", shares,
"total_shares", escrowTotalSharesActive,
"total_balance", escrowBalanceActive,
"err", err,
)
}
d.Shares = shares

ds.Delegations = append(ds.Delegations, d)
Expand Down Expand Up @@ -956,7 +985,7 @@ func (c *StorageClient) DebondingDelegations(ctx context.Context, address stakin
Delegator: address.String(),
}
var shares, escrowBalanceDebonding, escrowTotalSharesDebonding common.BigInt
if err := res.rows.Scan(
if err = res.rows.Scan(
&d.Validator,
&shares,
&d.DebondEnd,
Expand All @@ -965,7 +994,17 @@ func (c *StorageClient) DebondingDelegations(ctx context.Context, address stakin
); err != nil {
return nil, wrapError(err)
}
d.Amount = amountFromShares(shares, escrowTotalSharesDebonding, escrowBalanceDebonding)
d.Amount, err = amountFromShares(shares, escrowTotalSharesDebonding, escrowBalanceDebonding)
if err != nil {
c.logger.Error("failed to compute debonding delegated amount from shares (inconsistent DB state?)",
"delegator", address.String(),
"delegatee", d.Validator,
"shares", shares,
"total_shares_debonding", escrowTotalSharesDebonding,
"total_balance_debonding", escrowBalanceDebonding,
"err", err,
)
}
d.Shares = shares

ds.DebondingDelegations = append(ds.DebondingDelegations, d)
Expand Down Expand Up @@ -998,7 +1037,7 @@ func (c *StorageClient) DebondingDelegationsTo(ctx context.Context, address stak
Validator: address.String(),
}
var shares, escrowBalanceDebonding, escrowTotalSharesDebonding common.BigInt
if err := res.rows.Scan(
if err = res.rows.Scan(
&d.Delegator,
&shares,
&d.DebondEnd,
Expand All @@ -1007,7 +1046,17 @@ func (c *StorageClient) DebondingDelegationsTo(ctx context.Context, address stak
); err != nil {
return nil, wrapError(err)
}
d.Amount = amountFromShares(shares, escrowTotalSharesDebonding, escrowBalanceDebonding)
d.Amount, err = amountFromShares(shares, escrowTotalSharesDebonding, escrowBalanceDebonding)
if err != nil {
c.logger.Error("failed to compute debonding delegated amount from shares (inconsistent DB state?)",
"delegator", address.String(),
"delegatee", d.Validator,
"shares", shares,
"total_shares_debonding", escrowTotalSharesDebonding,
"total_balance_debonding", escrowBalanceDebonding,
"err", err,
)
}
d.Shares = shares

ds.DebondingDelegations = append(ds.DebondingDelegations, d)
Expand Down
Loading