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

Jump to bottom

transfer (total escrow): add escrow / unescrow functions #3507

Merged
merged 7 commits into from
May 3, 2023
Merged

transfer (total escrow): add escrow / unescrow functions #3507

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b65a52c
add escrow/unescrow functions
crodriguezvega Apr 22, 2023
eb11388
fix
crodriguezvega Apr 22, 2023
332d235
fix linter warning
crodriguezvega Apr 22, 2023
889aa6d
imp: add more code documentation
colin-axner Apr 27, 2023
310144e
Merge branch 'main' into carlos/add-escrow-unescrow-functions
colin-axner Apr 27, 2023
63cbe67
Merge branch 'main' into carlos/add-escrow-unescrow-functions
DimitrisJim Apr 27, 2023
49c81c1
chore: restructure code to match sybling function
colin-axner May 3, 2023
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
77 changes: 41 additions & 36 deletions modules/apps/transfer/keeper/relay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,12 @@ func (k Keeper) sendTransfer(
if types.SenderChainIsSource(sourcePort, sourceChannel, fullDenomPath) {
labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, "true"))

// create the escrow address for the tokens
// obtain the escrow address for the source channel end
escrowAddress := types.GetEscrowAddress(sourcePort, sourceChannel)

// escrow source tokens. It fails if balance insufficient.
if err := k.bankKeeper.SendCoins(
ctx, sender, escrowAddress, sdk.NewCoins(token),
); err != nil {
if err := k.escrowToken(ctx, sender, escrowAddress, token); err != nil {
return 0, err
}

// track the total amount in escrow keyed by denomination to allow for efficient iteration
currentTotalEscrow := k.GetTotalEscrowForDenom(ctx, token.GetDenom())
newTotalEscrow := currentTotalEscrow.Add(token.Amount)
k.SetTotalEscrowForDenom(ctx, token.GetDenom(), newTotalEscrow)
} else {
labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, "false"))

Expand Down Expand Up @@ -224,21 +216,11 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to receive funds", receiver)
}

// unescrow tokens
escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel())
if err := k.bankKeeper.SendCoins(ctx, escrowAddress, receiver, sdk.NewCoins(token)); err != nil {
// NOTE: this error is only expected to occur given an unexpected bug or a malicious
// counterparty module. The bug may occur in bank or any part of the code that allows
// the escrow address to be drained. A malicious counterparty module could drain the
// escrow address by allowing more tokens to be sent back then were escrowed.
return errorsmod.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module")
if err := k.unescrowToken(ctx, escrowAddress, receiver, token); err != nil {
return err
}

// track the total amount in escrow keyed by denomination to allow for efficient iteration
currentTotalEscrow := k.GetTotalEscrowForDenom(ctx, token.GetDenom())
newTotalEscrow := currentTotalEscrow.Sub(token.Amount)
k.SetTotalEscrowForDenom(ctx, token.GetDenom(), newTotalEscrow)

defer func() {
if transferAmount.IsInt64() {
telemetry.SetGaugeWithLabels(
Expand Down Expand Up @@ -367,20 +349,7 @@ func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, d
if types.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) {
// unescrow tokens back to sender
escrowAddress := types.GetEscrowAddress(packet.GetSourcePort(), packet.GetSourceChannel())
if err := k.bankKeeper.SendCoins(ctx, escrowAddress, sender, sdk.NewCoins(token)); err != nil {
// NOTE: this error is only expected to occur given an unexpected bug or a malicious
// counterparty module. The bug may occur in bank or any part of the code that allows
// the escrow address to be drained. A malicious counterparty module could drain the
// escrow address by allowing more tokens to be sent back then were escrowed.
return errorsmod.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module")
}

// track the total amount in escrow keyed by denomination to allow for efficient iteration
currentTotalEscrow := k.GetTotalEscrowForDenom(ctx, token.GetDenom())
newTotalEscrow := currentTotalEscrow.Sub(token.Amount)
k.SetTotalEscrowForDenom(ctx, token.GetDenom(), newTotalEscrow)

return nil
return k.unescrowToken(ctx, escrowAddress, sender, token)
}

// mint vouchers back to sender
Expand All @@ -397,6 +366,42 @@ func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, d
return nil
}

// escrowToken will send the given token from the provided sender to the escrow address. It will also
// update the total escrowed amount by adding the escrowed token to the current total escrow.
func (k Keeper) escrowToken(ctx sdk.Context, sender, escrowAddress sdk.AccAddress, token sdk.Coin) error {
if err := k.bankKeeper.SendCoins(
ctx, sender, escrowAddress, sdk.NewCoins(token),
); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

nit; I think we should put this on one line similarly to below in unescrowToken

Copy link
Contributor

@colin-axner colin-axner May 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I thought about the same, agreed! I think it helps to make the functions as identical as possible

return err // failure is expected for insufficient balances
}

// track the total amount in escrow keyed by denomination to allow for efficient iteration
currentTotalEscrow := k.GetTotalEscrowForDenom(ctx, token.GetDenom())
newTotalEscrow := currentTotalEscrow.Add(token.Amount)
k.SetTotalEscrowForDenom(ctx, token.GetDenom(), newTotalEscrow)

return nil
}

// unescrowToken will send the given token from the escrow address to the provided receiver. It will also
// update the total escrow by deducting the unescrowed token from the current total escrow.
func (k Keeper) unescrowToken(ctx sdk.Context, escrowAddress, receiver sdk.AccAddress, token sdk.Coin) error {
if err := k.bankKeeper.SendCoins(ctx, escrowAddress, receiver, sdk.NewCoins(token)); err != nil {
// NOTE: this error is only expected to occur given an unexpected bug or a malicious
// counterparty module. The bug may occur in bank or any part of the code that allows
// the escrow address to be drained. A malicious counterparty module could drain the
// escrow address by allowing more tokens to be sent back then were escrowed.
return errorsmod.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module")
}

// track the total amount in escrow keyed by denomination to allow for efficient iteration
currentTotalEscrow := k.GetTotalEscrowForDenom(ctx, token.GetDenom())
newTotalEscrow := currentTotalEscrow.Sub(token.Amount)
k.SetTotalEscrowForDenom(ctx, token.GetDenom(), newTotalEscrow)

return nil
}

// DenomPathFromHash returns the full denomination path prefix from an ibc denom with a hash
// component.
func (k Keeper) DenomPathFromHash(ctx sdk.Context, denom string) (string, error) {
Expand Down