From 73f875b3d4d8f1054cb9ec907b90d31cd716ad2f Mon Sep 17 00:00:00 2001 From: Joe Bowman Date: Tue, 13 Feb 2024 19:20:55 +0000 Subject: [PATCH] Handle non staking tokens (#1132) * test: update versions of external tools * Always use helpers to increment/decrement/set withdrawal waitgroup in order for under/overflow checks * fix: race condition in non-staking denom reward dist * add non-staking denom test to bash tests * improve validateCoinsForZone, and handle non-staking coins airdropped to delegate account * test: add delegate account airdrops test to bash tests * fix: SendToWithdrawal shouldn't use fixed sender * Ensure we have the same cases for failed messages * fix: handle race condition in delegation record update * update port for testzone * lint * Update x/interchainstaking/keeper/ibc_packet_handlers.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * tigy logging * run tests with go 1.22 * update golang * bump golangci-lint * update Makefile for go 1.22 * Update Makefile Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Jacob Gadikian --- .github/workflows/buildtest.yaml | 2 +- .github/workflows/codeql.yml | 2 +- .github/workflows/golangci.yml | 4 +- .github/workflows/interchaintest.yaml | 6 +- Dockerfile | 4 +- Dockerfile.hermes | 20 -- Dockerfile.relayer | 2 +- Makefile | 3 +- docker-compose.yml | 18 +- icq-relayer/Dockerfile | 2 +- proto/Dockerfile | 2 +- scripts/config/hermes.toml | 9 +- scripts/setup.sh | 4 +- scripts/simple-test.sh | 19 +- scripts/vars.sh | 6 +- x/interchainstaking/keeper/callbacks.go | 42 ++- x/interchainstaking/keeper/callbacks_test.go | 312 ++++++++++++++++-- x/interchainstaking/keeper/delegation.go | 19 +- x/interchainstaking/keeper/delegation_test.go | 32 +- x/interchainstaking/keeper/hooks.go | 21 +- .../keeper/ibc_packet_handlers.go | 107 +++--- .../keeper/ibc_packet_handlers_test.go | 168 +++++++++- x/interchainstaking/keeper/intent.go | 2 +- x/interchainstaking/keeper/keeper.go | 14 + x/interchainstaking/keeper/receipt.go | 34 +- x/interchainstaking/keeper/receipt_test.go | 2 +- x/interchainstaking/keeper/validators.go | 10 + .../keeper/withdrawal_record.go | 2 +- x/interchainstaking/transfer_middleware.go | 19 +- x/interchainstaking/types/zones.go | 58 +++- x/interchainstaking/types/zones_test.go | 42 ++- 31 files changed, 767 insertions(+), 220 deletions(-) delete mode 100644 Dockerfile.hermes diff --git a/.github/workflows/buildtest.yaml b/.github/workflows/buildtest.yaml index f99e15013..caa7be62a 100644 --- a/.github/workflows/buildtest.yaml +++ b/.github/workflows/buildtest.yaml @@ -75,7 +75,7 @@ jobs: strategy: matrix: arch: [amd64] - targetos: [linux, windows, darwin] + targetos: [linux, windows] include: - targetos: darwin arch: arm64 diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 945ab33e8..29635ff57 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -26,7 +26,7 @@ jobs: uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.21.5 + go-version: 1.21.7 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL uses: github/codeql-action/init@v3 diff --git a/.github/workflows/golangci.yml b/.github/workflows/golangci.yml index 4158f8f20..ded3c71dc 100644 --- a/.github/workflows/golangci.yml +++ b/.github/workflows/golangci.yml @@ -27,7 +27,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '1.21' + go-version: '1.21.x' cache: false - name: golangci-lint uses: golangci/golangci-lint-action@v4 @@ -35,7 +35,7 @@ jobs: # Require: The version of golangci-lint to use. # When `install-mode` is `binary` (default) the value can be v1.2 or v1.2.3 or `latest` to use the latest version. # When `install-mode` is `goinstall` the value can be v1.2.3, `latest`, or the hash of a commit. - version: v1.55.2 + version: v1.56.1 args: --timeout 15m # Optional: working directory, useful for monorepos # working-directory: somedir diff --git a/.github/workflows/interchaintest.yaml b/.github/workflows/interchaintest.yaml index 8c600726f..38c3545f8 100644 --- a/.github/workflows/interchaintest.yaml +++ b/.github/workflows/interchaintest.yaml @@ -55,7 +55,7 @@ jobs: runs-on: ubuntu-latest needs: build-and-push-image steps: - - name: Set up go 1.21 + - name: Set up go uses: actions/setup-go@v5 with: go-version: "1.21" @@ -72,7 +72,7 @@ jobs: runs-on: ubuntu-latest needs: build-and-push-image steps: - - name: Set up go 1.21 + - name: Set up go uses: actions/setup-go@v5 with: go-version: "1.21" @@ -89,7 +89,7 @@ jobs: runs-on: ubuntu-latest needs: build-and-push-image steps: - - name: Set up go 1.21 + - name: Set up go uses: actions/setup-go@v5 with: go-version: "1.21" diff --git a/Dockerfile b/Dockerfile index 15d00fcb0..91eefffeb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.21.5-alpine3.18 AS builder +FROM golang:1.21.7-alpine3.19 AS builder RUN apk add --no-cache git musl-dev openssl-dev linux-headers ca-certificates build-base WORKDIR /src/app/ @@ -22,7 +22,7 @@ RUN --mount=type=cache,target=/root/.cache/go-build \ LINK_STATICALLY=true make build # Add to a distroless container -FROM alpine:3.18 +FROM alpine:3.19 COPY --from=builder /src/app/build/quicksilverd /usr/local/bin/quicksilverd RUN adduser -S -h /quicksilver -D quicksilver -u 1000 USER quicksilver diff --git a/Dockerfile.hermes b/Dockerfile.hermes deleted file mode 100644 index fd18654a7..000000000 --- a/Dockerfile.hermes +++ /dev/null @@ -1,20 +0,0 @@ -FROM rust:1.72 as build -ARG VERSION - -RUN apt update && apt install git -y - -WORKDIR /app/src - -RUN git clone https://github.com/informalsystems/ibc-rs --branch $VERSION - -WORKDIR ibc-rs - -RUN cargo build --release - -FROM debian:bullseye-slim - -RUN apt update && apt install ca-certificates -y -COPY --from=build /app/src/ibc-rs/target/release/hermes /usr/local/bin/hermes -RUN adduser --system --home /hermes --disabled-password --disabled-login hermes -u 1000 -USER hermes - diff --git a/Dockerfile.relayer b/Dockerfile.relayer index 39a13872c..fab36ae31 100644 --- a/Dockerfile.relayer +++ b/Dockerfile.relayer @@ -1,4 +1,4 @@ -FROM golang:1.21.5-alpine3.18 AS builder +FROM golang:1.21.7-alpine3.19 AS builder RUN apk add --no-cache make git gcc musl-dev openssl-dev linux-headers RUN git clone https://github.com/cosmos/relayer --branch v2.1.1 /src/app diff --git a/Makefile b/Makefile index 8e73e44a6..3f61215ff 100755 --- a/Makefile +++ b/Makefile @@ -130,7 +130,8 @@ BUILD_TARGETS := build install check_version: ifneq ($(GO_MINOR_VERSION),21) - @echo "ERROR: Go version 1.21 is required for building Quicksilver. There are consensus breaking changes between binaries compiled with different Go versions." + @echo "ERROR: Go version 1.21 is required for building Quicksilver. Detected version: $(GO_MAJOR_VERSION).$(GO_MINOR_VERSION). There are +consensus breaking changes between binaries compiled with different Go versions." exit 1 endif diff --git a/docker-compose.yml b/docker-compose.yml index ae22d142a..1d0175d4f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -37,7 +37,7 @@ services: - start ports: - 27657:26657 - - 23137:1317 + - 21317:1317 testzone1-2: image: quicksilverzone/testzone:latest hostname: testzone1-2 @@ -98,31 +98,27 @@ services: - osmosisd - start hermes: - image: quicksilverzone/hermes:v1.5.0 + image: informalsystems/hermes:v1.8.0 hostname: hermes volumes: - - ./data/hermes:/hermes/.hermes + - ./data/hermes:/home/hermes/.hermes command: - - hermes - start restart: always - build: - context: . - dockerfile: Dockerfile.hermes icq: - image: quicksilverzone/interchain-queries:v0.9.1 + image: quicksilverzone/interchain-queries:v0.10.0 volumes: - ./data/icq:/icq/.icq command: - - interchain-queries + - icq-relayer - run restart: always icq2: - image: quicksilverzone/interchain-queries:v0.9.1 + image: quicksilverzone/interchain-queries:v0.10.0 volumes: - ./data/icq2:/icq/.icq command: - - interchain-queries + - icq-relayer - run restart: always relayer: diff --git a/icq-relayer/Dockerfile b/icq-relayer/Dockerfile index 93a52471d..78af0e456 100644 --- a/icq-relayer/Dockerfile +++ b/icq-relayer/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.21.5-alpine3.19 as build +FROM golang:1.21.7-alpine3.19 as build WORKDIR /src/app diff --git a/proto/Dockerfile b/proto/Dockerfile index 30a4baee5..2a437433c 100644 --- a/proto/Dockerfile +++ b/proto/Dockerfile @@ -1,5 +1,5 @@ FROM bufbuild/buf:latest as BUILDER -FROM golang:1.21.5-alpine3.18 +FROM golang:1.21.7-alpine3.18 RUN apk add --no-cache \ nodejs \ diff --git a/scripts/config/hermes.toml b/scripts/config/hermes.toml index 40248933f..158efff99 100644 --- a/scripts/config/hermes.toml +++ b/scripts/config/hermes.toml @@ -14,6 +14,7 @@ enabled = true enabled = true clear_interval = 100 clear_on_start = true +tx_confirmation = true [rest] enabled = true @@ -29,7 +30,7 @@ port = 3001 id = 'qstest-1' rpc_addr = 'http://quicksilver:26657' grpc_addr = 'http://quicksilver:9090' -websocket_addr = 'ws://quicksilver:26657/websocket' +event_source = { mode = "pull" } rpc_timeout = '10s' account_prefix = 'quick' key_name = 'testkey' @@ -45,12 +46,14 @@ max_block_time = '10s' trusting_period = '3minutes' trust_threshold = { numerator = '1', denominator = '3' } address_type = { derivation = 'cosmos' } +trusted_node = true +sequential_batch_tx = true [[chains]] id = 'lstest-1' rpc_addr = 'http://testzone1-1:26657' grpc_addr = 'http://testzone1-1:9090' -websocket_addr = 'ws://testzone1-1:26657/websocket' +event_source = { mode = "pull" } rpc_timeout = '10s' account_prefix = 'cosmos' key_name = 'testkey' @@ -66,3 +69,5 @@ max_block_time = '10s' trusting_period = '3minutes' trust_threshold = { numerator = '1', denominator = '3' } address_type = { derivation = 'cosmos' } +trusted_node = true +sequential_batch_tx = true diff --git a/scripts/setup.sh b/scripts/setup.sh index dbc0a2296..d68640d9b 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -266,7 +266,7 @@ $QS1_RUN add-genesis-account ${DEMO_ADDRESS_7} 100000000000uqck $QS2_RUN add-genesis-account ${VAL_ADDRESS_6} 100000000000uqck $QS3_RUN add-genesis-account ${VAL_ADDRESS_7} 100000000000uqck -$TZ1_1_RUN add-genesis-account ${VAL_ADDRESS_2} 100000000000uatom +$TZ1_1_RUN add-genesis-account ${VAL_ADDRESS_2} 100000000000uatom,100000000uother $TZ1_1_RUN add-genesis-account ${VAL_ADDRESS_3} 100000000000uatom $TZ1_1_RUN add-genesis-account ${VAL_ADDRESS_4} 100000000000uatom $TZ1_1_RUN add-genesis-account ${VAL_ADDRESS_5} 100000000000uatom @@ -475,7 +475,7 @@ if [ "$IS_MULTI_ZONE_TEST" = true ]; then fi ## set the 'epoch' epoch to 5m interval -jq '.app_state.epochs.epochs = [{"identifier": "epoch","start_time": "0001-01-01T00:00:00Z","duration": "240s","current_epoch": "0","current_epoch_start_time": "0001-01-01T00:00:00Z","epoch_counting_started": false,"current_epoch_start_height": "0"}]' ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json > ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json.new && mv ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json{.new,} +jq '.app_state.epochs.epochs = [{"identifier": "epoch","start_time": "0001-01-01T00:00:00Z","duration": "360s","current_epoch": "0","current_epoch_start_time": "0001-01-01T00:00:00Z","epoch_counting_started": false,"current_epoch_start_height": "0"},{"identifier": "day","start_time": "0001-01-01T00:00:00Z","duration": "120s","current_epoch": "0","current_epoch_start_time": "0001-01-01T00:00:00Z","epoch_counting_started": false,"current_epoch_start_height": "0"}]' ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json > ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json.new && mv ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json{.new,} jq '.app_state.interchainstaking.params.deposit_interval = 25' ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json > ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json.new && mv ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json{.new,} jq '.app_state.mint.params.epoch_identifier = "epoch"' ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json > ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json.new && mv ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json{.new,} jq '.app_state.gov.deposit_params.min_deposit = [{"denom": "uqck", "amount": "100"}]' ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json > ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json.new && mv ./${CHAIN_DIR}/${CHAINID_0}/config/genesis.json{.new,} diff --git a/scripts/simple-test.sh b/scripts/simple-test.sh index 0850f3724..8144bf1f5 100755 --- a/scripts/simple-test.sh +++ b/scripts/simple-test.sh @@ -108,8 +108,19 @@ while [[ "$PERFORMANCE_ACCOUNT" == "null" ]]; do PERFORMANCE_ACCOUNT=$($QS1_EXEC q interchainstaking zones --output=json | jq .zones[0].performance_address.address -r) done -$TZ1_1_EXEC tx bank send val2 $PERFORMANCE_ACCOUNT 40000uatom --chain-id $CHAINID_1 -y --keyring-backend=test +WITHDRAWAL_ACCOUNT=$($QS1_EXEC q interchainstaking zones --output=json | jq .zones[0].withdrawal_address.address -r) +while [[ "$WITHDRAWAL_ACCOUNT" == "null" ]]; do + sleep 2 + WITHDRAWAL_ACCOUNT=$($QS1_EXEC q interchainstaking zones --output=json | jq .zones[0].withdrawal_address.address -r) +done + +DELEGATE_ACCOUNT=$($QS1_EXEC q interchainstaking zones --output=json | jq .zones[0].delegation_address.address -r) +while [[ "DELEGATE_ACCOUNT" == "null" ]]; do + sleep 2 + DELEGATE_ACCOUNT=$($QS1_EXEC q interchainstaking zones --output=json | jq .zones[0].delegation_address.address -r) +done +$TZ1_1_EXEC tx bank send val2 $PERFORMANCE_ACCOUNT 40000uatom --chain-id $CHAINID_1 -y --keyring-backend=test sleep 3 #$TZ1_1_EXEC tx bank send val2 $DEPOSIT_ACCOUNT 10000000${VAL_VALOPER_2}1 --chain-id $CHAINID_1 -y --keyring-backend=test #sleep 5 @@ -117,6 +128,12 @@ sleep 3 #sleep 5 $TZ1_1_EXEC tx bank send demowallet2 $DEPOSIT_ACCOUNT 333333uatom --chain-id $CHAINID_1 -y --keyring-backend=test sleep 5 +$TZ1_1_EXEC tx bank send val2 $DEPOSIT_ACCOUNT 5000000uother --chain-id $CHAINID_1 -y --keyring-backend=test +sleep 5 +$TZ1_1_EXEC tx bank send val2 $WITHDRAWAL_ACCOUNT 8000000uother --chain-id $CHAINID_1 -y --keyring-backend=test +sleep 5 +$TZ1_1_EXEC tx bank send val2 $DELEGATE_ACCOUNT 10000000uother --chain-id $CHAINID_1 -y --keyring-backend=test +sleep 5 $TZ1_1_EXEC tx bank send demowallet2 $DEPOSIT_ACCOUNT 20000000uatom --chain-id $CHAINID_1 -y --keyring-backend=test --note MgTUzEjWVVYoDZBarqFL1akb38mxlgTsqdZ/sFxTJBNf+tv6rtckvn3T sleep 5 $TZ1_1_EXEC tx bank send demowallet2 $DEPOSIT_ACCOUNT 33000000uatom --chain-id $CHAINID_1 -y --keyring-backend=test diff --git a/scripts/vars.sh b/scripts/vars.sh index 6b4f7f76c..8623af525 100755 --- a/scripts/vars.sh +++ b/scripts/vars.sh @@ -47,7 +47,7 @@ TZ2_2_RUN="docker-compose $DC --ansi never run --rm -T testzone2-2 osmosisd" TZ2_3_RUN="docker-compose $DC --ansi never run --rm -T testzone2-3 osmosisd" TZ2_4_RUN="docker-compose $DC --ansi never run --rm -T testzone2-4 osmosisd" RLY_RUN="docker-compose $DC --ansi never run --rm -T relayer rly" -HERMES_RUN="docker-compose $DC --ansi never run --rm -T hermes hermes" +HERMES_RUN="docker-compose $DC --ansi never run --rm -T hermes" QS1_EXEC="docker-compose $DC --ansi never exec -T quicksilver quicksilverd" QS2_EXEC="docker-compose $DC --ansi never exec -T quicksilver2 quicksilverd" @@ -65,8 +65,8 @@ TZ2_3_EXEC="docker-compose $DC --ansi never exec -T testzone2-3 osmosisd" TZ2_4_EXEC="docker-compose $DC --ansi never exec -T testzone2-4 osmosisd" RLY_EXEC="docker-compose $DC --ansi never exec -T relayer" -ICQ_RUN="docker-compose $DC --ansi never run --rm -T icq interchain-queries" -ICQ2_RUN="docker-compose $DC --ansi never run --rm -T icq2 interchain-queries" +ICQ_RUN="docker-compose $DC --ansi never run --rm -T icq icq-relayer" +ICQ2_RUN="docker-compose $DC --ansi never run --rm -T icq2 icq-relayer" VAL_MNEMONIC_1="clock post desk civil pottery foster expand merit dash seminar song memory figure uniform spice circle try happy obvious trash crime hybrid hood cushion" VAL_MNEMONIC_2="angry twist harsh drastic left brass behave host shove marriage fall update business leg direct reward object ugly security warm tuna model broccoli choice" diff --git a/x/interchainstaking/keeper/callbacks.go b/x/interchainstaking/keeper/callbacks.go index ffc65be4d..9518cca66 100644 --- a/x/interchainstaking/keeper/callbacks.go +++ b/x/interchainstaking/keeper/callbacks.go @@ -124,12 +124,12 @@ func RewardsCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes.Que // decrement waitgroup as we have received back the query // (initially incremented in AfterEpochEnd) - err = zone.DecrementWithdrawalWaitgroup() + err = zone.DecrementWithdrawalWaitgroup(k.Logger(ctx), 1, "rewards callback") if err != nil { return err } - k.Logger(ctx).Debug("QueryDelegationRewards callback", "wg", zone.WithdrawalWaitgroup, "delegatorAddress", rewardsQuery.DelegatorAddress, "zone", query.ChainId) + k.Logger(ctx).Debug("QueryDelegationRewards callback", "wg", zone.GetWithdrawalWaitgroup(), "delegatorAddress", rewardsQuery.DelegatorAddress, "zone", query.ChainId) return k.WithdrawDelegationRewardsForResponse(ctx, &zone, rewardsQuery.DelegatorAddress, args) } @@ -291,7 +291,7 @@ func SigningInfoCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes return fmt.Errorf("can not get validator address from consensus address: %s", valSigningInfo.Address) } - k.Logger(ctx).Error("Tombstoned validator found", "valoper", valAddr) + k.Logger(ctx).Info("tombstoned validator found", "valoper", valAddr) valAddrBytes, err := addressutils.ValAddressFromBech32(valAddr, zone.GetValoperPrefix()) if err != nil { @@ -622,7 +622,7 @@ func DelegationAccountBalanceCallback(k *Keeper, ctx sdk.Context, args []byte, q } k.Logger(ctx).Info("Received balance response for denom", "denom", coin.Denom) - err = zone.DecrementWithdrawalWaitgroup() + err = zone.DecrementWithdrawalWaitgroup(k.Logger(ctx), 1, "delegationaccountbalance callback") if err != nil { return err } @@ -642,6 +642,12 @@ func DelegationAccountBalanceCallback(k *Keeper, ctx sdk.Context, args []byte, q k.SetZone(ctx, &zone) + // if token is not valid for staking, then send to withdrawal account. + if valid, _ := zone.ValidateCoinsForZone(sdk.NewCoins(coin), k.GetValidatorAddressesAsMap(ctx, zone.ChainId)); !valid { + k.Logger(ctx).Info("token is not a valid staking token, so sending to withdrawal account for disbursal", "chain", zone.ChainId, "assets", coin) + return k.SendToWithdrawal(ctx, &zone, zone.DelegationAddress, sdk.NewCoins(coin)) + } + return k.FlushOutstandingDelegations(ctx, &zone, coin) } @@ -653,7 +659,9 @@ func DelegationAccountBalancesCallback(k *Keeper, ctx sdk.Context, args []byte, result := banktypes.QueryAllBalancesResponse{} k.cdc.MustUnmarshal(args, &result) - zone.WithdrawalWaitgroup-- + if err := zone.DecrementWithdrawalWaitgroup(k.Logger(ctx), 1, "delegationaccountbalances callback"); err != nil { + return err + } addressBytes, err := addressutils.AccAddressFromBech32(zone.DelegationAddress.Address, zone.AccountPrefix) if err != nil { @@ -677,8 +685,10 @@ func DelegationAccountBalancesCallback(k *Keeper, ctx sdk.Context, args []byte, 0, ) - k.Logger(ctx).Info("Emitting balance request for denom", "denom", coin.Denom) - zone.WithdrawalWaitgroup++ + if err = zone.IncrementWithdrawalWaitgroup(k.Logger(ctx), 1, fmt.Sprintf("delegation account balance for %s", coin.Denom)); err != nil { + return err + } + k.Logger(ctx).Info("Emitting balance request for denom", "denom", coin.Denom, "waitgroup", zone.GetWithdrawalWaitgroup()) } k.SetZone(ctx, &zone) @@ -708,22 +718,22 @@ func AllBalancesCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes case zone.DepositAddress != nil && balanceQuery.Address == zone.DepositAddress.Address: if zone.DepositAddress.BalanceWaitgroup != 0 { zone.DepositAddress.BalanceWaitgroup = 0 - k.Logger(ctx).Error("Zeroing deposit balance waitgroup") + k.Logger(ctx).Info("zeroing deposit balance waitgroup") } case zone.WithdrawalAddress != nil && balanceQuery.Address == zone.WithdrawalAddress.Address: if zone.WithdrawalAddress.BalanceWaitgroup != 0 { zone.WithdrawalAddress.BalanceWaitgroup = 0 - k.Logger(ctx).Error("Zeroing withdrawal balance waitgroup") + k.Logger(ctx).Info("zeroing withdrawal balance waitgroup") } case zone.DelegationAddress != nil && balanceQuery.Address == zone.DelegationAddress.Address: if zone.DelegationAddress.BalanceWaitgroup != 0 { zone.DelegationAddress.BalanceWaitgroup = 0 - k.Logger(ctx).Error("Zeroing delegation balance waitgroup") + k.Logger(ctx).Info("zeroing delegation balance waitgroup") } case zone.PerformanceAddress != nil && balanceQuery.Address == zone.PerformanceAddress.Address: if zone.PerformanceAddress.BalanceWaitgroup != 0 { zone.PerformanceAddress.BalanceWaitgroup = 0 - k.Logger(ctx).Error("Zeroing performance balance waitgroup") + k.Logger(ctx).Info("zeroing performance balance waitgroup") } } k.SetZone(ctx, &zone) @@ -735,7 +745,7 @@ func AllBalancesCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes func TxDecoder(cdc codec.Codec) sdk.TxDecoder { return func(txBytes []byte) (sdk.Tx, error) { // Make sure txBytes follow ADR-027. - err := rejectNonADR027TxRaw(txBytes) + err := RejectNonADR027TxRaw(txBytes) if err != nil { return nil, sdkioerrors.Wrap(sdkerrors.ErrTxDecode, err.Error()) } @@ -769,7 +779,7 @@ func TxDecoder(cdc codec.Codec) sdk.TxDecoder { } } -func rejectNonADR027TxRaw(txBytes []byte) error { +func RejectNonADR027TxRaw(txBytes []byte) error { // Make sure all fields are ordered in ascending order with this variable. prevTagNum := protowire.Number(0) @@ -797,7 +807,7 @@ func rejectNonADR027TxRaw(txBytes []byte) error { return fmt.Errorf("invalid length; %w", protowire.ParseError(m)) } // We make sure that this varint is as short as possible. - n := varintMinLength(lengthPrefix) + n := VarintMinLength(lengthPrefix) if n != m { return fmt.Errorf("length prefix varint for tagNum %d is not as short as possible, read %d, only need %d", tagNum, m, n) } @@ -813,9 +823,9 @@ func rejectNonADR027TxRaw(txBytes []byte) error { return nil } -// varintMinLength returns the minimum number of bytes necessary to encode an +// VarintMinLength returns the minimum number of bytes necessary to encode an // uint using varint encoding. -func varintMinLength(n uint64) int { +func VarintMinLength(n uint64) int { switch { // Note: 1< 0 { - k.Logger(ctx).Error( - "epoch waitgroup was unexpected > 0; this means we did not process the previous epoch!", - "chain_id", zone.ChainId, - "epoch_identifier", epochIdentifier, - "epoch_number", epochNumber, - ) - zone.WithdrawalWaitgroup = 0 + if zone.GetWithdrawalWaitgroup() > 0 { + zone.SetWithdrawalWaitgroup(k.Logger(ctx), 0, "epoch waitgroup was unexpected > 0") } // OnChanOpenAck calls SetWithdrawalAddress (see ibc_module.go) @@ -178,7 +172,7 @@ func (k *Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNum 0, ) // increment waitgroup; decremented in delegationaccountbalance callback - zone.WithdrawalWaitgroup++ + _ = zone.IncrementWithdrawalWaitgroup(k.Logger(ctx), 1, "delegationaccountbalances trigger") rewardsQuery := distrtypes.QueryDelegationTotalRewardsRequest{DelegatorAddress: zone.DelegationAddress.Address} bz = k.cdc.MustMarshal(&rewardsQuery) @@ -198,13 +192,8 @@ func (k *Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNum // increment the WithdrawalWaitgroup // this allows us to track the response for every protocol delegator // WithdrawalWaitgroup is decremented in RewardsCallback - zone.WithdrawalWaitgroup++ - k.Logger(ctx).Info("Incrementing waitgroup for delegation", - "value", zone.WithdrawalWaitgroup, - "chain_id", zone.ChainId, - "epoch_identifier", epochIdentifier, - "epoch_number", epochNumber, - ) + _ = zone.IncrementWithdrawalWaitgroup(k.Logger(ctx), 1, "rewards trigger") + k.SetZone(ctx, zone) return false diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 7acf318a9..58cb7decd 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -130,7 +130,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack if !ok { return errors.New("unable to unmarshal MsgWithdrawDelegatorReward") } - k.Logger(ctx).Error("Failed to withdraw rewards; will try again next epoch", "validator", withdrawalMsg.ValidatorAddress) + k.Logger(ctx).Error("failed to withdraw rewards; will try again next epoch", "validator", withdrawalMsg.ValidatorAddress) return nil } k.Logger(ctx).Info("Rewards withdrawn") @@ -313,7 +313,6 @@ func (k *Keeper) HandleMsgTransfer(ctx sdk.Context, msg ibctransfertypes.Fungibl } if found && msg.Denom != zone.BaseDenom { - // k.Logger(ctx).Error("got withdrawal account and NOT staking denom", "rx", receivedCoin.Denom, "trace_base_denom", denomTrace.BaseDenom, "zone_base_denom", zone.BaseDenom) feeAmount := sdk.NewDecFromInt(receivedCoin.Amount).Mul(k.GetCommissionRate(ctx)).TruncateInt() rewardCoin := receivedCoin.SubAmount(feeAmount) zoneAddress, err := addressutils.AccAddressFromBech32(zone.WithdrawalAddress.Address, "") @@ -353,15 +352,23 @@ func (k *Keeper) HandleCompleteSend(ctx sdk.Context, msg sdk.Msg, memo string) e // checks here are specific to ensure future extensibility; switch { - case zone.IsWithdrawalAddress(sMsg.FromAddress): - // WithdrawalAddress (for rewards) only send to DelegationAddresses. - // Target here is the DelegationAddresses. + case zone.IsDelegateAddress(sMsg.ToAddress) && zone.IsWithdrawalAddress(sMsg.FromAddress): + k.Logger(ctx).Info("delegate account received tokens from withdrawal account; delegating rewards", "amount", sMsg.Amount) return k.handleRewardsDelegation(ctx, *zone, sMsg) - case zone.IsDelegateAddress(sMsg.FromAddress): - return k.HandleWithdrawForUser(ctx, zone, sMsg, memo) - case zone.IsDelegateAddress(sMsg.ToAddress) && zone.DepositAddress.Address == sMsg.FromAddress: + + case zone.IsWithdrawalAddress(sMsg.ToAddress): + k.Logger(ctx).Info("withdrawal account received tokens to disburse", "amount", sMsg.Amount) + return nil + + case zone.IsDelegateAddress(sMsg.ToAddress) && zone.IsDepositAddress(sMsg.FromAddress): + k.Logger(ctx).Info("delegate account received tokens from deposit account; delegating deposit", "amount", sMsg.Amount, "memo", memo) _, err := k.handleSendToDelegate(ctx, zone, sMsg, memo) return err + + case zone.IsDelegateAddress(sMsg.FromAddress): + k.Logger(ctx).Info("delegate account send tokens; handling withdrawal", "amount", sMsg.Amount, "memo", memo) + return k.HandleWithdrawForUser(ctx, zone, sMsg, memo) + default: err = fmt.Errorf("unexpected completed send (2) from %s to %s (amount: %s)", sMsg.FromAddress, sMsg.ToAddress, sMsg.Amount) k.Logger(ctx).Error(err.Error()) @@ -568,11 +575,18 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion // a zero completion time can only happen when the validator is unbonded; this means the redelegation has _already_ completed and can be removed. k.DeleteRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) } else { - record, found := k.GetRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) if !found { - k.Logger(ctx).Error("unable to find redelegation record", "chain", zone.ChainId, "source", redelegateMsg.ValidatorSrcAddress, "dst", redelegateMsg.ValidatorDstAddress, "epoch_number", epochNumber) - return fmt.Errorf("unable to find redelegation record for chain %s, src: %s, dst: %s, at epoch %d", zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) + // it is possible that the record was cleaned up if there was a long delay in processing acknowledgements. + // just create a new one + record = types.RedelegationRecord{ + ChainId: zone.ChainId, + EpochNumber: epochNumber, + Source: redelegateMsg.ValidatorSrcAddress, + Destination: redelegateMsg.ValidatorDstAddress, + Amount: redelegateMsg.Amount.Amount.Int64(), + CompletionTime: completion, + } } k.Logger(ctx).Info("updating redelegation record with completion time", "completion", completion) @@ -618,9 +632,18 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion k.Logger(ctx).Error("unable to find delegation record", "chain", zone.ChainId, "source", redelegateMsg.ValidatorSrcAddress, "dst", redelegateMsg.ValidatorDstAddress, "epoch_number", epochNumber) return fmt.Errorf("unable to find delegation record for chain %s, src: %s, dst: %s, at epoch %d", zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) } - srcDelegation.Amount = srcDelegation.Amount.Sub(redelegateMsg.Amount) - - k.SetDelegation(ctx, zone.ChainId, srcDelegation) + srcDelegation.Amount, err = srcDelegation.Amount.SafeSub(redelegateMsg.Amount) + if err != nil { + if strings.Contains(err.Error(), "negative coin amount") { + // we received a negative srcDelegation. Obviously this cannot happen, but we can get a crossed re/un/delegation, all which fetch absolute values. + k.Logger(ctx).Error("possible race condition; unable to sub redelegation amount. requerying delegation anyway") + } else { + // we got some other, unrecoverable err + return err + } + } else { + k.SetDelegation(ctx, zone.ChainId, srcDelegation) + } valAddr, err = addressutils.ValAddressFromBech32(redelegateMsg.ValidatorDstAddress, zone.AccountPrefix+"valoper") if err != nil { @@ -649,7 +672,7 @@ func (k *Keeper) HandleFailedBeginRedelegate(ctx sdk.Context, msg sdk.Msg, memo return err } - k.Logger(ctx).Error("Received MsgBeginRedelegate acknowledgement error") + k.Logger(ctx).Error("received MsgBeginRedelegate acknowledgement error") // first, type assertion. we should have stakingtypes.MsgBeginRedelegate redelegateMsg, ok := msg.(*stakingtypes.MsgBeginRedelegate) if !ok { @@ -660,7 +683,7 @@ func (k *Keeper) HandleFailedBeginRedelegate(ctx sdk.Context, msg sdk.Msg, memo return fmt.Errorf("zone for delegate account %s not found", redelegateMsg.DelegatorAddress) } k.DeleteRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) - k.Logger(ctx).Error("Cleaning up redelegation record") + k.Logger(ctx).Info("cleaning up redelegation record") return nil } @@ -748,18 +771,20 @@ func (k *Keeper) HandleFailedBankSend(ctx sdk.Context, msg sdk.Msg, memo string) // checks here are specific to ensure future extensibility; switch { - case zone.IsWithdrawalAddress(sMsg.FromAddress): + case zone.IsDelegateAddress(sMsg.ToAddress) && zone.IsWithdrawalAddress(sMsg.FromAddress): // MsgSend from Withdrawal account to delegate account was not completed. We can ignore this. - k.Logger(ctx).Error("MsgSend from withdrawal account to delegate account failed") + k.Logger(ctx).Info("MsgSend to delegate account from withdrawal account failed", "amount", sMsg.Amount) + case zone.IsWithdrawalAddress(sMsg.ToAddress): + k.Logger(ctx).Info("MsgSend to withdrawal account for disbursal failed", "amount", sMsg.Amount) + case zone.IsDelegateAddress(sMsg.ToAddress) && zone.IsDepositAddress(sMsg.FromAddress): + // MsgSend from deposit account to delegate account for deposit. + k.Logger(ctx).Error("MsgSend from deposit account to delegate account failed", "amount", sMsg.Amount) case zone.IsDelegateAddress(sMsg.FromAddress): + k.Logger(ctx).Info("MsgSend from delegate account failed; updating withdrawal", "amount", sMsg.Amount, "memo", memo) return k.HandleFailedUnbondSend(ctx, sMsg, memo) - case zone.IsDelegateAddress(sMsg.ToAddress) && zone.DepositAddress.Address == sMsg.FromAddress: - // MsgSend from deposit account to delegate account for deposit. - k.Logger(ctx).Error("MsgSend from deposit account to delegate account failed") default: - err = fmt.Errorf("unexpected completed send (1) from %s to %s (amount: %s)", sMsg.FromAddress, sMsg.ToAddress, sMsg.Amount) + err = fmt.Errorf("unexpected failed send (1) from %s to %s (amount: %s)", sMsg.FromAddress, sMsg.ToAddress, sMsg.Amount) k.Logger(ctx).Error(err.Error()) - return nil } return nil @@ -795,7 +820,7 @@ func (k *Keeper) HandleFailedUndelegate(ctx sdk.Context, msg sdk.Msg, memo strin return err } - k.Logger(ctx).Error("Received MsgUndelegate acknowledgement error") + k.Logger(ctx).Error("received MsgUndelegate acknowledgement error") // first, type assertion. we should have stakingtypes.MsgBeginRedelegate undelegateMsg, ok := msg.(*stakingtypes.MsgUndelegate) if !ok { @@ -864,7 +889,7 @@ func (k *Keeper) HandleFailedUndelegate(ctx sdk.Context, msg sdk.Msg, memo strin } k.DeleteUnbondingRecord(ctx, zone.ChainId, undelegateMsg.ValidatorAddress, epochNumber) - k.Logger(ctx).Error("Cleaning up redelegation record") + k.Logger(ctx).Info("cleaning up unbonding record") return nil } @@ -896,7 +921,7 @@ func (k *Keeper) HandleRedeemTokens(ctx sdk.Context, msg sdk.Msg, amount sdk.Coi k.SetReceiptsCompleted(ctx, zone.ChainId, time.Unix(exclusionTimestampUnix, 0), ctx.BlockTime(), redeemMsg.Amount.Denom) zone.DelegationAddress.Balance = zone.DelegationAddress.Balance.Sub(redeemMsg.Amount) k.SetZone(ctx, zone) - if zone.WithdrawalWaitgroup == 0 { + if zone.GetWithdrawalWaitgroup() == 0 { k.Logger(ctx).Info("Triggering redemption rate calc after delegation flush") if err = k.TriggerRedemptionRate(ctx, zone); err != nil { return err @@ -935,9 +960,12 @@ func (k *Keeper) HandleFailedRedeemTokens(ctx sdk.Context, msg sdk.Msg, memo str switch { case strings.HasPrefix(memo, "batch"): k.Logger(ctx).Error("batch token redemption failed", "memo", memo, "tx", redeemMsg) - zone.WithdrawalWaitgroup-- + if err := zone.DecrementWithdrawalWaitgroup(k.Logger(ctx), uint32(1), "batch token redemption failure ack"); err != nil { + return err + } + k.Logger(ctx).Info("Decremented waitgroup after failed batch token redemption", "wg", zone.GetWithdrawalWaitgroup()) k.SetZone(ctx, zone) - if zone.WithdrawalWaitgroup == 0 { + if zone.GetWithdrawalWaitgroup() == 0 { k.Logger(ctx).Info("Triggering redemption rate calc after delegation flush") if err := k.TriggerRedemptionRate(ctx, zone); err != nil { return err @@ -969,7 +997,7 @@ func (k *Keeper) HandleDelegate(ctx sdk.Context, msg sdk.Msg, memo string) error switch { case memo == "rewards": case strings.HasPrefix(memo, "batch"): - k.Logger(ctx).Error("batch delegation", "memo", memo, "tx", delegateMsg) + k.Logger(ctx).Info("batch delegation", "memo", memo, "tx", delegateMsg) exclusionTimestampUnix, err := strconv.ParseInt(strings.Split(memo, "/")[1], 10, 64) if err != nil { return err @@ -977,9 +1005,11 @@ func (k *Keeper) HandleDelegate(ctx sdk.Context, msg sdk.Msg, memo string) error k.Logger(ctx).Debug("outstanding delegations ack-received") k.SetReceiptsCompleted(ctx, zone.ChainId, time.Unix(exclusionTimestampUnix, 0), ctx.BlockTime(), delegateMsg.Amount.Denom) zone.DelegationAddress.Balance = zone.DelegationAddress.Balance.Sub(delegateMsg.Amount) - zone.WithdrawalWaitgroup-- + if err := zone.DecrementWithdrawalWaitgroup(k.Logger(ctx), uint32(1), "batch/reward delegation success ack"); err != nil { + return err + } k.SetZone(ctx, zone) - if zone.WithdrawalWaitgroup == 0 { + if zone.GetWithdrawalWaitgroup() == 0 { k.Logger(ctx).Info("Triggering redemption rate calc after delegation flush") if err := k.TriggerRedemptionRate(ctx, zone); err != nil { return err @@ -1019,9 +1049,11 @@ func (k *Keeper) HandleFailedDelegate(ctx sdk.Context, msg sdk.Msg, memo string) switch { case strings.HasPrefix(memo, "batch"): k.Logger(ctx).Error("batch delegation failed", "memo", memo, "tx", delegateMsg) - zone.WithdrawalWaitgroup-- + if err := zone.DecrementWithdrawalWaitgroup(k.Logger(ctx), 1, "batch delegation failed ack"); err != nil { + return err + } k.SetZone(ctx, zone) - if zone.WithdrawalWaitgroup == 0 { + if zone.GetWithdrawalWaitgroup() == 0 { k.Logger(ctx).Info("Triggering redemption rate calc after delegation flush") if err := k.TriggerRedemptionRate(ctx, zone); err != nil { return err @@ -1211,15 +1243,16 @@ func (k *Keeper) HandleWithdrawRewards(ctx sdk.Context, msg sdk.Msg) error { // operates outside the delegator set, its purpose is to track validator // performance only. if withdrawalMsg.DelegatorAddress != zone.PerformanceAddress.Address { - err = zone.DecrementWithdrawalWaitgroup() + if err := zone.DecrementWithdrawalWaitgroup(k.Logger(ctx), 1, "handle withdraw rewards"); err != nil { + return err + } if err != nil { return err } - k.Logger(ctx).Info("Decremented waitgroup", "wg", zone.WithdrawalWaitgroup) k.SetZone(ctx, zone) } - k.Logger(ctx).Info("Received MsgWithdrawDelegatorReward acknowledgement", "wg", zone.WithdrawalWaitgroup, "delegator", withdrawalMsg.DelegatorAddress) - switch zone.WithdrawalWaitgroup == 0 { + k.Logger(ctx).Info("Received MsgWithdrawDelegatorReward acknowledgement", "wg", zone.GetWithdrawalWaitgroup(), "delegator", withdrawalMsg.DelegatorAddress) + switch zone.GetWithdrawalWaitgroup() == 0 { case true: k.Logger(ctx).Info("triggering redemption rate calc after rewards withdrawal") return k.TriggerRedemptionRate(ctx, zone) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers_test.go b/x/interchainstaking/keeper/ibc_packet_handlers_test.go index 18bb8b89e..19a67ab71 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers_test.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers_test.go @@ -813,7 +813,7 @@ func (suite *KeeperTestSuite) TestHandleWithdrawRewards() { { name: "try to decrement when waitgroup = 0", setup: func(ctx sdk.Context, quicksilver *app.Quicksilver, zone *types.Zone) { - zone.WithdrawalWaitgroup = 0 + zone.SetWithdrawalWaitgroup(quicksilver.Logger(), 0, "init") quicksilver.InterchainstakingKeeper.SetZone(ctx, zone) }, checks: func(ctx sdk.Context, quicksilver *app.Quicksilver, zone *types.Zone) {}, @@ -829,7 +829,8 @@ func (suite *KeeperTestSuite) TestHandleWithdrawRewards() { { name: "valid case with balances != 0", setup: func(ctx sdk.Context, quicksilver *app.Quicksilver, zone *types.Zone) { - zone.WithdrawalWaitgroup = 1 + zone.SetWithdrawalWaitgroup(quicksilver.Logger(), 1, "init") + balances := sdk.NewCoins( sdk.NewCoin( zone.BaseDenom, @@ -852,13 +853,13 @@ func (suite *KeeperTestSuite) TestHandleWithdrawRewards() { { name: "valid case trigger redemption rate and check if delegatorAddress == performanceAddress", setup: func(ctx sdk.Context, quicksilver *app.Quicksilver, zone *types.Zone) { - zone.WithdrawalWaitgroup = 1 + zone.SetWithdrawalWaitgroup(quicksilver.Logger(), 1, "init") quicksilver.InterchainstakingKeeper.SetZone(ctx, zone) }, checks: func(ctx sdk.Context, quicksilver *app.Quicksilver, zone *types.Zone) { newZone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, zone.ChainId) suite.True(found) - suite.Zero(newZone.WithdrawalWaitgroup) + suite.Zero(newZone.GetWithdrawalWaitgroup()) }, msg: func(zone *types.Zone) sdk.Msg { return &distrtypes.MsgWithdrawDelegatorReward{ @@ -872,7 +873,7 @@ func (suite *KeeperTestSuite) TestHandleWithdrawRewards() { { name: "valid case trigger redemption rate and without checking if delegatorAddress == performanceAddress", setup: func(ctx sdk.Context, quicksilver *app.Quicksilver, zone *types.Zone) { - zone.WithdrawalWaitgroup = 0 + zone.SetWithdrawalWaitgroup(quicksilver.Logger(), 0, "init") quicksilver.InterchainstakingKeeper.SetZone(ctx, zone) }, checks: func(ctx sdk.Context, quicksilver *app.Quicksilver, zone *types.Zone) {}, @@ -2656,6 +2657,91 @@ func (suite *KeeperTestSuite) TestReceiveAckForBeginRedelegateNilCompletion() { suite.Equal(beforeTarget.Amount.Add(redelegate.Amount), afterTarget.Amount) } +func (suite *KeeperTestSuite) TestReceiveAckForBeginRedelegateNoExistingRecord() { + suite.SetupTest() + suite.setupTestZones() + + epoch := int64(2) + + quicksilver := suite.GetQuicksilverApp(suite.chainA) + ctx := suite.chainA.GetContext() + complete := ctx.BlockTime().Add(time.Hour * 72) + + zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + if !found { + suite.Fail("unable to retrieve zone for test") + } + validators := quicksilver.InterchainstakingKeeper.GetValidators(ctx, zone.ChainId) + + beforeTarget := types.Delegation{ + DelegationAddress: zone.DelegationAddress.Address, + ValidatorAddress: validators[1].ValoperAddress, + Amount: sdk.NewCoin(zone.BaseDenom, math.NewInt(2000)), + } + + beforeSource := types.Delegation{ + DelegationAddress: zone.DelegationAddress.Address, + ValidatorAddress: validators[0].ValoperAddress, + Amount: sdk.NewCoin(zone.BaseDenom, math.NewInt(1001)), + } + + quicksilver.InterchainstakingKeeper.SetDelegation(ctx, zone.ChainId, beforeTarget) + quicksilver.InterchainstakingKeeper.SetDelegation(ctx, zone.ChainId, beforeSource) + + redelegate := &stakingtypes.MsgBeginRedelegate{DelegatorAddress: zone.DelegationAddress.Address, ValidatorSrcAddress: validators[0].ValoperAddress, ValidatorDstAddress: validators[1].ValoperAddress, Amount: sdk.NewCoin(zone.BaseDenom, sdk.NewInt(1000))} + data, err := icatypes.SerializeCosmosTx(quicksilver.InterchainstakingKeeper.GetCodec(), []sdk.Msg{redelegate}) + suite.NoError(err) + + // validate memo < 256 bytes + packetData := icatypes.InterchainAccountPacketData{ + Type: icatypes.EXECUTE_TX, + Data: data, + Memo: types.EpochRebalanceMemo(epoch), + } + + packet := channeltypes.Packet{Data: quicksilver.InterchainstakingKeeper.GetCodec().MustMarshalJSON(&packetData)} + + response := stakingtypes.MsgUndelegateResponse{ + CompletionTime: complete, + } + + anyResponse, err := codectypes.NewAnyWithValue(&response) + suite.NoError(err) + + txMsgData := &sdk.TxMsgData{ + MsgResponses: []*codectypes.Any{anyResponse}, + } + + ackData := icatypes.ModuleCdc.MustMarshal(txMsgData) + + acknowledgement := channeltypes.NewResultAcknowledgement(ackData) + ackBytes, err := icatypes.ModuleCdc.MarshalJSON(&acknowledgement) + suite.NoError(err) + + // call handler + + err = quicksilver.InterchainstakingKeeper.HandleAcknowledgement(ctx, packet, ackBytes) + suite.NoError(err) + + createdRecord, found := quicksilver.InterchainstakingKeeper.GetRedelegationRecord(ctx, zone.ChainId, validators[0].ValoperAddress, validators[1].ValoperAddress, epoch) + suite.True(found) // redelegation record should have been removed. + + suite.Equal(redelegate.Amount.Amount.Int64(), createdRecord.Amount) + suite.Equal(redelegate.ValidatorDstAddress, createdRecord.Destination) + suite.Equal(redelegate.ValidatorSrcAddress, createdRecord.Source) + suite.Equal(epoch, createdRecord.EpochNumber) + suite.Equal(complete, createdRecord.CompletionTime) + + afterSource, found := quicksilver.InterchainstakingKeeper.GetDelegation(ctx, zone.ChainId, zone.DelegationAddress.Address, validators[0].ValoperAddress) + suite.True(found) + suite.Equal(beforeSource.Amount.Sub(redelegate.Amount), afterSource.Amount) + + afterTarget, found := quicksilver.InterchainstakingKeeper.GetDelegation(ctx, zone.ChainId, zone.DelegationAddress.Address, validators[1].ValoperAddress) + suite.True(found) + suite.Equal(complete.Unix(), afterTarget.RedelegationEnd) + suite.Equal(beforeTarget.Amount.Add(redelegate.Amount), afterTarget.Amount) +} + func (suite *KeeperTestSuite) TestReceiveAckForWithdrawReward() { suite.SetupTest() suite.setupTestZones() @@ -2670,7 +2756,7 @@ func (suite *KeeperTestSuite) TestReceiveAckForWithdrawReward() { if !found { suite.Fail("unable to retrieve zone for test") } - zone.WithdrawalWaitgroup = 1 + zone.SetWithdrawalWaitgroup(quicksilver.Logger(), 1, "init") quicksilver.InterchainstakingKeeper.SetZone(ctx, &zone) withdrawReward := &distrtypes.MsgWithdrawDelegatorReward{ @@ -4020,7 +4106,7 @@ func (suite *KeeperTestSuite) TestHandleCompleteSend() { message: func(zone *types.Zone) sdk.Msg { return &banktypes.MsgSend{ FromAddress: zone.WithdrawalAddress.Address, - ToAddress: "", + ToAddress: zone.DelegationAddress.Address, Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(1_000_000))), } }, @@ -4050,6 +4136,30 @@ func (suite *KeeperTestSuite) TestHandleCompleteSend() { memo: "unbondSend/7C8B95EEE82CB63771E02EBEB05E6A80076D70B2E0A1C457F1FD1A0EF2EA961D", expectedError: errors.New("no matching withdrawal record found"), }, + { + name: "From DepositAddress to Withdrawal Address", + message: func(zone *types.Zone) sdk.Msg { + return &banktypes.MsgSend{ + FromAddress: zone.DepositAddress.Address, + ToAddress: zone.WithdrawalAddress.Address, + Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(1_000_000))), + } + }, + memo: "", + expectedError: nil, + }, + { + name: "From DelegateAddress to Withdrawal Address", + message: func(zone *types.Zone) sdk.Msg { + return &banktypes.MsgSend{ + FromAddress: zone.DelegationAddress.Address, + ToAddress: zone.WithdrawalAddress.Address, + Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(1_000_000))), + } + }, + memo: "", + expectedError: nil, + }, } for _, tc := range testCases { @@ -4479,7 +4589,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_Batch_OK() { zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) - zone.WithdrawalWaitgroup = 100 + zone.SetWithdrawalWaitgroup(app.Logger(), 100, "init") app.InterchainstakingKeeper.SetZone(ctx, &zone) vals := app.InterchainstakingKeeper.GetValidatorAddresses(ctx, suite.chainB.ChainID) @@ -4490,7 +4600,31 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_Batch_OK() { zone, found = app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) - suite.Equal(uint32(99), zone.WithdrawalWaitgroup) + suite.Equal(uint32(99), zone.GetWithdrawalWaitgroup()) +} + +func (suite *KeeperTestSuite) TestHandleFailedDelegate_Batch_BadWg() { + suite.SetupTest() + suite.setupTestZones() + + app := suite.GetQuicksilverApp(suite.chainA) + ctx := suite.chainA.GetContext() + + zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + suite.True(found) + + zone.SetWithdrawalWaitgroup(app.Logger(), 0, "init") + app.InterchainstakingKeeper.SetZone(ctx, &zone) + + vals := app.InterchainstakingKeeper.GetValidatorAddresses(ctx, suite.chainB.ChainID) + msg := stakingtypes.MsgDelegate{DelegatorAddress: zone.DelegationAddress.Address, ValidatorAddress: vals[0], Amount: sdk.NewCoin("uatom", sdk.NewInt(100))} + var msgMsg sdk.Msg = &msg + err := app.InterchainstakingKeeper.HandleFailedDelegate(ctx, msgMsg, "batch/12345678") + suite.Error(err) + + zone, found = app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + suite.True(found) + suite.Equal(uint32(0), zone.GetWithdrawalWaitgroup()) } func (suite *KeeperTestSuite) TestHandleFailedDelegate_PerfAddress_OK() { @@ -4503,7 +4637,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_PerfAddress_OK() { zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) - zone.WithdrawalWaitgroup = 100 + zone.SetWithdrawalWaitgroup(app.Logger(), 100, "init") app.InterchainstakingKeeper.SetZone(ctx, &zone) vals := app.InterchainstakingKeeper.GetValidatorAddresses(ctx, suite.chainB.ChainID) @@ -4515,7 +4649,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_PerfAddress_OK() { zone, found = app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) // delegator was perf address, no change in waitgroup - suite.Equal(uint32(100), zone.WithdrawalWaitgroup) + suite.Equal(uint32(100), zone.GetWithdrawalWaitgroup()) } func (suite *KeeperTestSuite) TestHandleFailedDelegate_NotBatch_OK() { @@ -4528,7 +4662,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_NotBatch_OK() { zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) - zone.WithdrawalWaitgroup = 100 + zone.SetWithdrawalWaitgroup(app.Logger(), 100, "init") app.InterchainstakingKeeper.SetZone(ctx, &zone) vals := app.InterchainstakingKeeper.GetValidatorAddresses(ctx, suite.chainB.ChainID) @@ -4540,7 +4674,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_NotBatch_OK() { zone, found = app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) // memo was not a batch id, so don't decrement withdrawal wg - suite.Equal(uint32(100), zone.WithdrawalWaitgroup) + suite.Equal(uint32(100), zone.GetWithdrawalWaitgroup()) } func (suite *KeeperTestSuite) TestHandleFailedDelegate_BatchTriggerRR_OK() { @@ -4553,7 +4687,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_BatchTriggerRR_OK() { zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) - zone.WithdrawalWaitgroup = 1 + zone.SetWithdrawalWaitgroup(app.Logger(), 1, "init") app.InterchainstakingKeeper.SetZone(ctx, &zone) preQueries := app.InterchainQueryKeeper.AllQueries(ctx) @@ -4566,7 +4700,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_BatchTriggerRR_OK() { zone, found = app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) // memo was not a batch id, so don't decrement withdrawal wg - suite.Equal(uint32(0), zone.WithdrawalWaitgroup) + suite.Equal(uint32(0), zone.GetWithdrawalWaitgroup()) postQueries := app.InterchainQueryKeeper.AllQueries(ctx) @@ -4600,7 +4734,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_BadAddr_Fail() { zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) - zone.WithdrawalWaitgroup = 100 + zone.SetWithdrawalWaitgroup(app.Logger(), 100, "init") app.InterchainstakingKeeper.SetZone(ctx, &zone) vals := app.InterchainstakingKeeper.GetValidatorAddresses(ctx, suite.chainB.ChainID) @@ -4620,7 +4754,7 @@ func (suite *KeeperTestSuite) TestHandleFailedDelegate_BadMsg_Fail() { zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) suite.True(found) - zone.WithdrawalWaitgroup = 100 + zone.SetWithdrawalWaitgroup(app.Logger(), 100, "init") app.InterchainstakingKeeper.SetZone(ctx, &zone) vals := app.InterchainstakingKeeper.GetValidatorAddresses(ctx, suite.chainB.ChainID) diff --git a/x/interchainstaking/keeper/intent.go b/x/interchainstaking/keeper/intent.go index 96eb2c5be..b503f2ff9 100644 --- a/x/interchainstaking/keeper/intent.go +++ b/x/interchainstaking/keeper/intent.go @@ -203,7 +203,7 @@ func (k *Keeper) UpdateDelegatorIntent(ctx sdk.Context, delegator sdk.AccAddress // grab offchain asset value, and raise the users' base value by this amount. k.ClaimsManagerKeeper.IterateLastEpochUserClaims(ctx, zone.ChainId, delegator.String(), func(index int64, claim prtypes.Claim) (stop bool) { claimAmt = claimAmt.Add(sdkmath.NewIntFromUint64(claim.Amount)) - k.Logger(ctx).Error("Update intents - found claim for user", "user", delIntent.Delegator, "claim amount", claim.Amount, "new balance", claimAmt) + k.Logger(ctx).Info("Update intents - found claim for user", "user", delIntent.Delegator, "claim amount", claim.Amount, "new balance", claimAmt) return false }) diff --git a/x/interchainstaking/keeper/keeper.go b/x/interchainstaking/keeper/keeper.go index 4d0322702..c3477749a 100644 --- a/x/interchainstaking/keeper/keeper.go +++ b/x/interchainstaking/keeper/keeper.go @@ -800,3 +800,17 @@ func (k *Keeper) UnmarshalValidator(data []byte) (lsmstakingtypes.Validator, err return validator, nil } + +func (k *Keeper) SendToWithdrawal(ctx sdk.Context, zone *types.Zone, sender *types.ICAAccount, amount sdk.Coins) error { + var msgs []sdk.Msg + + sendMsg := banktypes.MsgSend{ + FromAddress: sender.Address, + ToAddress: zone.WithdrawalAddress.Address, + Amount: amount, + } + + msgs = append(msgs, &sendMsg) + + return k.SubmitTx(ctx, msgs, sender, "", zone.MessagesPerTx) +} diff --git a/x/interchainstaking/keeper/receipt.go b/x/interchainstaking/keeper/receipt.go index 624c22024..097a2fd4b 100644 --- a/x/interchainstaking/keeper/receipt.go +++ b/x/interchainstaking/keeper/receipt.go @@ -17,7 +17,6 @@ import ( channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v5/modules/core/24-host" - "github.com/quicksilver-zone/quicksilver/utils" "github.com/quicksilver-zone/quicksilver/utils/addressutils" "github.com/quicksilver-zone/quicksilver/x/interchainstaking/types" minttypes "github.com/quicksilver-zone/quicksilver/x/mint/types" @@ -51,9 +50,10 @@ func (k *Keeper) HandleReceiptTransaction(ctx sdk.Context, txn *tx.Tx, hash stri } if sender != senderAddress { + // TODO: technically nothing wrong with this; just need to make sure we _only_ consider assets sent to QS k.Logger(ctx).Error("sender mismatch", "expected", senderAddress, "received", sender) k.NilReceipt(ctx, &zone, hash) // nil receipt will stop this hash being submitted again - return nil + return k.SendToWithdrawal(ctx, &zone, zone.DepositAddress, assets) } k.Logger(ctx).Info("Deposit receipt", "deposit_address", zone.DepositAddress.GetAddress(), "sender", sender, "amount", amount) @@ -63,23 +63,32 @@ func (k *Keeper) HandleReceiptTransaction(ctx sdk.Context, txn *tx.Tx, hash stri } - if senderAddress == Unset { + if senderAddress == Unset { // not sure this is ever reachable. A valid MsgSend must always have a valid sender. k.Logger(ctx).Error("no sender found. Ignoring.") k.NilReceipt(ctx, &zone, hash) // nil receipt will stop this hash being submitted again - return nil + return k.SendToWithdrawal(ctx, &zone, zone.DepositAddress, assets) + } senderAccAddress, err := addressutils.AccAddressFromBech32(senderAddress, zone.GetAccountPrefix()) - if err != nil { + if err != nil { // not sure this is ever reachable. A valid MsgSend must always have a valid sender. k.Logger(ctx).Error("unable to decode sender address. Ignoring.", "senderAddress", senderAddress, "error", err) k.NilReceipt(ctx, &zone, hash) // nil receipt will stop this hash being submitted again - return nil + return k.SendToWithdrawal(ctx, &zone, zone.DepositAddress, assets) + } - if err := zone.ValidateCoinsForZone(assets, utils.StringSliceToMap(k.GetValidatorAddresses(ctx, zone.ChainId))); err != nil { - // we expect this to trigger if the validatorset has changed recently (i.e. we haven't seen the validator before. - // That is okay, we'll catch it next round!) + valid, matchesVals := zone.ValidateCoinsForZone(assets, k.GetValidatorAddressesAsMap(ctx, zone.ChainId)) + + if !valid { k.Logger(ctx).Error("unable to validate coins. Ignoring.", "senderAddress", senderAddress) - return fmt.Errorf("unable to validate coins. Ignoring. senderAddress=%q", senderAddress) + k.NewCompletedReceipt(ctx, &zone, senderAddress, hash, assets) // nil receipt will stop this hash being submitted again + // send tokens to withdrawal for disbursal. + return k.SendToWithdrawal(ctx, &zone, zone.DepositAddress, assets) + } else if !matchesVals { + k.Logger(ctx).Error("unable to validate coins for this valset.", "senderAddress", senderAddress) + // Do not set a nil receipt so we can revisit this tx. + // Don't return an error as to not clog queue. + return nil } k.Logger(ctx).Info("found new deposit tx", "deposit_address", zone.DepositAddress.GetAddress(), "senderAddress", senderAddress, "local", senderAccAddress.String(), "chain id", zone.ChainId, "assets", assets, "hash", hash) @@ -316,6 +325,11 @@ func (k Keeper) NilReceipt(ctx sdk.Context, zone *types.Zone, txhash string) { k.SetReceipt(ctx, r) } +func (Keeper) NewCompletedReceipt(ctx sdk.Context, zone *types.Zone, sender, txhash string, amount sdk.Coins) *types.Receipt { + t := ctx.BlockTime() + return &types.Receipt{ChainId: zone.ChainId, Sender: sender, Txhash: txhash, Amount: amount, FirstSeen: &t, Completed: &t} +} + func (Keeper) NewReceipt(ctx sdk.Context, zone *types.Zone, sender, txhash string, amount sdk.Coins) *types.Receipt { t := ctx.BlockTime() return &types.Receipt{ChainId: zone.ChainId, Sender: sender, Txhash: txhash, Amount: amount, FirstSeen: &t} diff --git a/x/interchainstaking/keeper/receipt_test.go b/x/interchainstaking/keeper/receipt_test.go index b7bd9739e..b1375149e 100644 --- a/x/interchainstaking/keeper/receipt_test.go +++ b/x/interchainstaking/keeper/receipt_test.go @@ -215,7 +215,7 @@ func (suite *KeeperTestSuite) TestHandleReceiptTransactionBadDenom() { suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.ZeroInt()), before) err = icsKeeper.HandleReceiptTransaction(ctx, transaction, hash, zone) - suite.ErrorContains(err, "unable to validate coins. Ignoring") + suite.NoError(err) after := suite.GetQuicksilverApp(suite.chainA).BankKeeper.GetSupply(ctx, zone.LocalDenom) suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.ZeroInt()), after) diff --git a/x/interchainstaking/keeper/validators.go b/x/interchainstaking/keeper/validators.go index 6282b45bb..ba3c20843 100644 --- a/x/interchainstaking/keeper/validators.go +++ b/x/interchainstaking/keeper/validators.go @@ -28,6 +28,16 @@ func (k Keeper) GetValidatorAddresses(ctx sdk.Context, chainID string) []string return validators } +// GetValidatorAddressesAsMap returns a slice of validator addresses by chainID. +func (k Keeper) GetValidatorAddressesAsMap(ctx sdk.Context, chainID string) map[string]bool { + validators := map[string]bool{} + k.IterateValidators(ctx, chainID, func(_ int64, validator types.Validator) (stop bool) { + validators[validator.ValoperAddress] = true + return false + }) + return validators +} + // GetActiveValidators returns validators by chainID where status = BONDED. func (k Keeper) GetActiveValidators(ctx sdk.Context, chainID string) []types.Validator { validators := []types.Validator{} diff --git a/x/interchainstaking/keeper/withdrawal_record.go b/x/interchainstaking/keeper/withdrawal_record.go index f18ab350f..8f504129d 100644 --- a/x/interchainstaking/keeper/withdrawal_record.go +++ b/x/interchainstaking/keeper/withdrawal_record.go @@ -31,7 +31,7 @@ func (k *Keeper) GetNextWithdrawalRecordSequence(ctx sdk.Context) (sequence uint func (k *Keeper) AddWithdrawalRecord(ctx sdk.Context, chainID, delegator string, distributions []*types.Distribution, recipient string, burnAmount sdk.Coin, hash string, status int32, completionTime time.Time, epochNumber int64) { record := types.WithdrawalRecord{ChainId: chainID, Delegator: delegator, Distribution: distributions, Recipient: recipient, Status: status, BurnAmount: burnAmount, Txhash: hash, CompletionTime: completionTime, EpochNumber: epochNumber} - k.Logger(ctx).Error("addWithdrawalRecord", "record", record) + k.Logger(ctx).Info("addWithdrawalRecord", "record", record) k.SetWithdrawalRecord(ctx, record) } diff --git a/x/interchainstaking/transfer_middleware.go b/x/interchainstaking/transfer_middleware.go index f66d9b2c4..772251ea5 100644 --- a/x/interchainstaking/transfer_middleware.go +++ b/x/interchainstaking/transfer_middleware.go @@ -93,18 +93,21 @@ func (im TransferMiddleware) OnRecvPacket( return channeltypes.NewErrorAcknowledgement(err) } - _, found := im.keeper.GetZoneForWithdrawalAccount(ctx, data.Sender) - if found { - if data.Receiver == im.keeper.AccountKeeper.GetModuleAddress(types.ModuleName).String() { - im.keeper.Logger(ctx).Info("msgTransfer to ics module account from withdrawal address") - err := im.keeper.HandleMsgTransfer(ctx, data, utils.DeriveIbcDenom(packet.DestinationPort, packet.DestinationChannel, packet.SourcePort, packet.SourceChannel, data.Denom)) - if err != nil { - im.keeper.Logger(ctx).Error("unable to disperse rewards", "error", err.Error()) + ack := im.app.OnRecvPacket(ctx, packet, relayer) + if ack.Success() { + _, found := im.keeper.GetZoneForWithdrawalAccount(ctx, data.Sender) + if found { + if data.Receiver == im.keeper.AccountKeeper.GetModuleAddress(types.ModuleName).String() { + im.keeper.Logger(ctx).Info("MsgTransfer to ics module account from withdrawal address") + err := im.keeper.HandleMsgTransfer(ctx, data, utils.DeriveIbcDenom(packet.DestinationPort, packet.DestinationChannel, packet.SourcePort, packet.SourceChannel, data.Denom)) + if err != nil { + im.keeper.Logger(ctx).Error("unable to disperse rewards", "error", err.Error()) + } } } } - return im.app.OnRecvPacket(ctx, packet, relayer) + return ack } // OnAcknowledgementPacket implements the IBCModule interface. diff --git a/x/interchainstaking/types/zones.go b/x/interchainstaking/types/zones.go index c1964a26e..e59748d74 100644 --- a/x/interchainstaking/types/zones.go +++ b/x/interchainstaking/types/zones.go @@ -6,6 +6,8 @@ import ( "fmt" "strings" + "github.com/tendermint/tendermint/libs/log" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/quicksilver-zone/quicksilver/utils/addressutils" @@ -22,6 +24,10 @@ func (z *Zone) GetValoperPrefix() string { return "" } +func (z Zone) IsDepositAddress(addr string) bool { + return z.DepositAddress != nil && z.DepositAddress.Address == addr +} + func (z Zone) IsDelegateAddress(addr string) bool { return z.DelegationAddress != nil && z.DelegationAddress.Address == addr } @@ -37,30 +43,60 @@ func (z *Zone) GetDelegationAccount() (*ICAAccount, error) { return z.DelegationAddress, nil } -func (z *Zone) DecrementWithdrawalWaitgroup() error { - if z.WithdrawalWaitgroup == 0 { +func (z *Zone) SetWithdrawalWaitgroup(logger log.Logger, num uint32, reason string) { + logger.Info("setting withdrawal waitgroup", "zone", z.ChainId, "existing", z.WithdrawalWaitgroup, "new", num, "reason", reason) + z.WithdrawalWaitgroup = num +} + +func (z *Zone) DecrementWithdrawalWaitgroup(logger log.Logger, num uint32, reason string) error { + if z.WithdrawalWaitgroup-num > z.WithdrawalWaitgroup { // underflow + logger.Error("error decrementing withdrawal waitgroup: uint32 underflow ", "zone", z.ChainId, "existing", z.WithdrawalWaitgroup, "decrement", num, "reason", reason) return errors.New("unable to decrement the withdrawal waitgroup below 0") } - z.WithdrawalWaitgroup-- + logger.Info("decrementing withdrawal waitgroup", "zone", z.ChainId, "existing", z.WithdrawalWaitgroup, "decrement", num, "new", z.WithdrawalWaitgroup-num, "reason", reason) + z.WithdrawalWaitgroup -= num + return nil +} + +func (z *Zone) IncrementWithdrawalWaitgroup(logger log.Logger, num uint32, reason string) error { + if z.WithdrawalWaitgroup+num < z.WithdrawalWaitgroup { // overflow + logger.Error("error incrementing withdrawal waitgroup: uint32 overflow ", "zone", z.ChainId, "existing", z.WithdrawalWaitgroup, "increment", num, "reason", reason) + return errors.New("unable to increment the withdrawal waitgroup above 4294967295") + } + logger.Info("incrementing withdrawal waitgroup", "zone", z.ChainId, "existing", z.WithdrawalWaitgroup, "increment", num, "new", z.WithdrawalWaitgroup+num, "reason", reason) + z.WithdrawalWaitgroup += num return nil } -func (z *Zone) ValidateCoinsForZone(coins sdk.Coins, zoneVals map[string]bool) error { +// ValidateCoinsForZone checks whether an inbound denomination is valid for this zone. +// valid coins comprise: +// - non-lsm: staking denom only (z.BaseDenom) +// - lsm: staking denom, and tokens prefixed with zone.AccountPrefix +// +// lsm: if valoper is not in zoneVals map, then we return true, false, and this tx will be revisited. +func (z *Zone) ValidateCoinsForZone(coins sdk.Coins, zoneVals map[string]bool) (valid bool, matchesVal bool) { for _, coin := range coins.Sort() { if coin.Denom == z.BaseDenom { continue } - coinParts := strings.Split(coin.Denom, "/") - if len(coinParts) != 2 { - return fmt.Errorf("invalid denom for zone: %s", coin.Denom) - } + // if liquidity module enabled, check to see if this is a tokenized share. + if z.LiquidityModule { + coinParts := strings.Split(coin.Denom, "/") + if len(coinParts) != 2 || !strings.HasPrefix(coinParts[0], z.AccountPrefix) { + return false, false + } - if _, ok := zoneVals[coinParts[0]]; !ok { - return fmt.Errorf("invalid denom for zone: %s", coin.Denom) + if _, ok := zoneVals[coinParts[0]]; !ok { + return true, false + } + + continue } + + return false, false } - return nil + return true, true } // memo functionality diff --git a/x/interchainstaking/types/zones_test.go b/x/interchainstaking/types/zones_test.go index 1e556597c..85e0690e1 100644 --- a/x/interchainstaking/types/zones_test.go +++ b/x/interchainstaking/types/zones_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/libs/log" sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -39,15 +40,16 @@ func TestGetDelegationAccount(t *testing.T) { } func TestDecrementWithdrawalWg(t *testing.T) { + testlog := log.NewNopLogger() zone := types.Zone{WithdrawalWaitgroup: 0} - oldWg := zone.WithdrawalWaitgroup - zone.WithdrawalWaitgroup++ - firstWg := zone.WithdrawalWaitgroup + oldWg := zone.GetWithdrawalWaitgroup() + require.NoError(t, zone.IncrementWithdrawalWaitgroup(testlog, 1, "test increment")) + firstWg := zone.GetWithdrawalWaitgroup() require.Equal(t, oldWg+1, firstWg) - require.NoError(t, zone.DecrementWithdrawalWaitgroup()) - secondWg := zone.WithdrawalWaitgroup + require.NoError(t, zone.DecrementWithdrawalWaitgroup(testlog, 1, "test decrement")) + secondWg := zone.GetWithdrawalWaitgroup() require.Equal(t, firstWg-1, secondWg) - require.Error(t, zone.DecrementWithdrawalWaitgroup()) + require.Error(t, zone.DecrementWithdrawalWaitgroup(testlog, 1, "test decrement error")) } func TestValidateCoinsForZone(t *testing.T) { @@ -59,8 +61,32 @@ func TestValidateCoinsForZone(t *testing.T) { "cosmosvaloper1a3yjj7d3qnx4spgvjcwjq9cw9snrrrhu5h6jll": true, "cosmosvaloper1z8zjv3lntpwxua0rtpvgrcwl0nm0tltgpgs6l7": true, } - require.NoError(t, zone.ValidateCoinsForZone(sdk.NewCoins(sdk.NewCoin("cosmosvaloper14lultfckehtszvzw4ehu0apvsr77afvyju5zzy/1", sdk.OneInt())), valAddresses)) - require.Errorf(t, zone.ValidateCoinsForZone(sdk.NewCoins(sdk.NewCoin("cosmosvaloper18ldc09yx4aua9g8mkl3sj526hgydzzyehcyjjr/1", sdk.OneInt())), valAddresses), "invalid denom for zone: cosmosvaloper18ldc09yx4aua9g8mkl3sj526hgydzzyehcyjjr/1") + + // valid AND matches a validator BUT lsm is off: false, false + valid, matches := zone.ValidateCoinsForZone(sdk.NewCoins(sdk.NewCoin("cosmosvaloper14lultfckehtszvzw4ehu0apvsr77afvyju5zzy/1", sdk.OneInt())), valAddresses) + require.False(t, valid) + require.False(t, matches) + + zone.LiquidityModule = true + // valid AND matches a validator: true, true + valid, matches = zone.ValidateCoinsForZone(sdk.NewCoins(sdk.NewCoin("cosmosvaloper14lultfckehtszvzw4ehu0apvsr77afvyju5zzy/1", sdk.OneInt())), valAddresses) + require.True(t, valid) + require.True(t, matches) + + // valid format but does not match: true, false + valid, matches = zone.ValidateCoinsForZone(sdk.NewCoins(sdk.NewCoin("cosmosvaloper18ldc09yx4aua9g8mkl3sj526hgydzzyehcyjjr/1", sdk.OneInt())), valAddresses) + require.True(t, valid) + require.False(t, matches) + + // invalid format (although valid ibc!) - false, false + valid, matches = zone.ValidateCoinsForZone(sdk.NewCoins(sdk.NewCoin("ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2", sdk.OneInt())), valAddresses) + require.False(t, valid) + require.False(t, matches) + + // staking token - true, true + valid, matches = zone.ValidateCoinsForZone(sdk.NewCoins(sdk.NewCoin("uatom", sdk.OneInt())), valAddresses) + require.True(t, valid) + require.True(t, matches) } func TestCoinsToIntent(t *testing.T) {