fix(twap/e2e): set geometric accum to zero; reproduce testnet bug (#3972

) (#3974)

* fix(twap/e2e): set geometric accum to zero; reproduce testnet bug

* test TestParseTwapFromBz

* clean up

(cherry picked from commit a693a3b)

Co-authored-by: Roman <roman@osmosis.team>

tests/e2e/e2e_test.go

@@ -3,14 +3,15 @@ package e2e
 import (
 	"encoding/json"
 	"fmt"
-	ibchookskeeper "github.com/osmosis-labs/osmosis/x/ibc-hooks/keeper"
 	"io"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
 
+	ibchookskeeper "github.com/osmosis-labs/osmosis/x/ibc-hooks/keeper"
+
 	paramsutils "github.com/cosmos/cosmos-sdk/x/params/client/utils"
 
 	ibcratelimittypes "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/types"
@@ -24,6 +25,42 @@ import (
 	"github.com/osmosis-labs/osmosis/v14/tests/e2e/initialization"
 )
 
+// TestGeometricTwapMigration tests that the geometric twap record
+// migration runs succesfully. It does so by attempting to execute
+// the swap on the pool created pre-upgrade. When a pool is created
+// pre-upgrade, twap records are initialized for a pool. By runnning
+// a swap post-upgrade, we confirm that the geometric twap was initialized
+// correctly and does not cause a chain halt. This test was created
+// in-response to a testnet incident when performing the geometric twap
+// upgrade. Upon adding the migrations logic, the tests began to pass.
+func (s *IntegrationTestSuite) TestGeometricTwapMigration() {
+	if s.skipUpgrade {
+		s.T().Skip("Skipping upgrade tests")
+	}
+
+	const (
+		// Configurations for tests/e2e/scripts/pool1A.json
+		// This pool gets initialized pre-upgrade.
+		oldPoolId       = 1
+		minAmountOut    = "1"
+		otherDenom      = "ibc/ED07A3391A112B175915CD8FAF43A2DA8E4790EDE12566649D0C2F97716B8518"
+		migrationWallet = "migration"
+	)
+
+	chainA := s.configurer.GetChainConfig(0)
+	node, err := chainA.GetDefaultNode()
+	s.Require().NoError(err)
+
+	uosmoIn := fmt.Sprintf("1000000%s", "uosmo")
+
+	swapWalletAddr := node.CreateWallet(migrationWallet)
+
+	node.BankSend(uosmoIn, chainA.NodeConfigs[0].PublicAddress, swapWalletAddr)
+
+	// Swap to create new twap records on the pool that was created pre-upgrade.
+	node.SwapExactAmountIn(uosmoIn, minAmountOut, fmt.Sprintf("%d", oldPoolId), otherDenom, swapWalletAddr)
+}
+
 // TestIBCTokenTransfer tests that IBC token transfers work as expected.
 // Additionally, it attempst to create a pool with IBC denoms.
 func (s *IntegrationTestSuite) TestIBCTokenTransferAndCreatePool() {

x/twap/types/keys.go

@@ -87,5 +87,8 @@ func ParseTwapFromBz(bz []byte) (twap TwapRecord, err error) {
 		return TwapRecord{}, errors.New("twap not found")
 	}
 	err = proto.Unmarshal(bz, &twap)
+	if twap.GeometricTwapAccumulator.IsNil() {
+		twap.GeometricTwapAccumulator = sdk.ZeroDec()
+	}
 	return twap, err
 }

x/twap/types/keys_test.go

@@ -61,28 +61,52 @@ func TestFormatHistoricalTwapKeys(t *testing.T) {
 
 func TestParseTwapFromBz(t *testing.T) {
 	baseTime := time.Unix(1257894000, 0).UTC()
+	baseParseRecord := TwapRecord{
+		PoolId:                      123,
+		Asset0Denom:                 "B",
+		Asset1Denom:                 "A",
+		Height:                      1,
+		Time:                        baseTime,
+		P0LastSpotPrice:             sdk.NewDecWithPrec(1, 5),
+		P1LastSpotPrice:             sdk.NewDecWithPrec(2, 5), // inconsistent value
+		P0ArithmeticTwapAccumulator: sdk.ZeroDec(),
+		P1ArithmeticTwapAccumulator: sdk.ZeroDec(),
+	}
+
+	withGeomAcc := func(r TwapRecord, acc sdk.Dec) TwapRecord {
+		r.GeometricTwapAccumulator = acc
+		return r
+	}
+
 	tests := map[string]struct {
-		record TwapRecord
+		record              TwapRecord
+		isGeometricAccumNil bool
 	}{
-		"standard": {TwapRecord{
-			PoolId:                      123,
-			Asset0Denom:                 "B",
-			Asset1Denom:                 "A",
-			Height:                      1,
-			Time:                        baseTime,
-			P0LastSpotPrice:             sdk.NewDecWithPrec(1, 5),
-			P1LastSpotPrice:             sdk.NewDecWithPrec(2, 5), // inconsistent value
-			P0ArithmeticTwapAccumulator: sdk.ZeroDec(),
-			P1ArithmeticTwapAccumulator: sdk.ZeroDec(),
-		}},
+		"standard": {
+			baseParseRecord,
+			false,
+		},
+		"with nil geometric twap accumulator -> set to zero": {
+			withGeomAcc(baseParseRecord, sdk.Dec{}),
+			true,
+		},
+		"with non-nil geometric twap accumulator -> not overwritten": {
+			withGeomAcc(baseParseRecord, sdk.OneDec()),
+			false,
+		},
 	}
 	for name, tt := range tests {
+		tt := tt
 		t.Run(name, func(t *testing.T) {
 			bz, err := proto.Marshal(&tt.record)
 			require.NoError(t, err)
 			record, err := ParseTwapFromBz(bz)
 			require.NoError(t, err)
 
+			if tt.isGeometricAccumNil {
+				tt.record.GeometricTwapAccumulator = sdk.ZeroDec()
+			}
+
 			require.Equal(t, tt.record, record)
 		})
 	}