refactor: replace timeout with height (#269)

Co-authored-by: nulnut <151493716+nulnut@users.noreply.github.com>
PundiAI · Mar 18, 2024 · 8db1a93 · 8db1a93
1 parent bbad8aa
commit 8db1a93
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 36 deletions.
diff --git a/app/genesis_test.go b/app/genesis_test.go
@@ -29,7 +29,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {
@@ -84,7 +84,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {
@@ -167,7 +167,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {
@@ -246,7 +246,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {
@@ -396,7 +396,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {
@@ -453,7 +453,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {
@@ -496,7 +496,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {
@@ -575,7 +575,7 @@ func TestNewDefaultGenesisByDenom(t *testing.T) {
 		"amount": "10000000000000000000000"
 	  },
 	  "delegate_multiple": "10",
-	  "bridge_call_refund_timeout": "604800000000000"
+	  "bridge_call_refund_timeout": "604800000"
 	},
 	"last_observed_event_nonce": "0",
 	"last_observed_block_height": {

diff --git a/solidity/contracts/bridge/FxBridgeLogic.sol b/solidity/contracts/bridge/FxBridgeLogic.sol
@@ -454,8 +454,8 @@ contract FxBridgeLogic is
             );
 
             require(
-                block.timestamp < _timeout,
-                "timeout must be greater than the current block timestamp."
+                block.number < _timeout,
+                "refund timeout must be greater than the current block height."
             );
 
             require(

diff --git a/x/crosschain/keeper/attestation.go b/x/crosschain/keeper/attestation.go
@@ -266,3 +266,22 @@ func (k Keeper) SetLastEventNonceByOracle(ctx sdk.Context, oracleAddr sdk.AccAdd
 	store := ctx.KVStore(k.storeKey)
 	store.Set(types.GetLastEventNonceByOracleKey(oracleAddr), sdk.Uint64ToBigEndian(eventNonce))
 }
+
+// CalExternalTimeoutHeight This gets the timeout height in External blocks.
+func (k Keeper) CalExternalTimeoutHeight(ctx sdk.Context, params types.Params, timeout uint64) uint64 {
+	currentFxHeight := ctx.BlockHeight()
+	// we store the last observed Cosmos and Ethereum heights, we do not concern ourselves if these values
+	// are zero because no batch can be produced if the last Ethereum block height is not first populated by a deposit event.
+	heights := k.GetLastObservedBlockHeight(ctx)
+	if heights.ExternalBlockHeight == 0 {
+		return 0
+	}
+	// we project how long it has been in milliseconds since the last Ethereum block height was observed
+	projectedMillis := (uint64(currentFxHeight) - heights.BlockHeight) * params.AverageBlockTime
+	// we convert that projection into the current Ethereum height using the average Ethereum block time in millis
+	projectedCurrentEthereumHeight := (projectedMillis / params.AverageExternalBlockTime) + heights.ExternalBlockHeight
+	// we convert our target time for block timeouts (lets say 12 hours) into a number of blocks to
+	// place on top of our projection of the current Ethereum block height.
+	blocksToAdd := timeout / params.AverageExternalBlockTime
+	return projectedCurrentEthereumHeight + blocksToAdd
+}
diff --git a/x/crosschain/keeper/batch.go b/x/crosschain/keeper/batch.go
@@ -40,7 +40,8 @@ func (k Keeper) BuildOutgoingTxBatch(ctx sdk.Context, tokenContract, feeReceive
 	if types.OutgoingTransferTxs(selectedTx).TotalFee().LT(minimumFee) {
 		return nil, errorsmod.Wrap(types.ErrInvalid, "total fee less than minimum fee")
 	}
-	batchTimeout := k.GetBatchTimeoutHeight(ctx)
+	params := k.GetParams(ctx)
+	batchTimeout := k.CalExternalTimeoutHeight(ctx, params, params.ExternalBatchTimeout)
 	if batchTimeout <= 0 {
 		return nil, errorsmod.Wrap(types.ErrInvalid, "batch timeout height")
 	}
@@ -79,26 +80,6 @@ func (k Keeper) BuildOutgoingTxBatch(ctx sdk.Context, tokenContract, feeReceive
 	return batch, nil
 }
 
-// GetBatchTimeoutHeight This gets the batch timeout height in External blocks.
-func (k Keeper) GetBatchTimeoutHeight(ctx sdk.Context) uint64 {
-	currentFxHeight := ctx.BlockHeight()
-	params := k.GetParams(ctx)
-	// we store the last observed Cosmos and Ethereum heights, we do not concern ourselves if these values
-	// are zero because no batch can be produced if the last Ethereum block height is not first populated by a deposit event.
-	heights := k.GetLastObservedBlockHeight(ctx)
-	if heights.ExternalBlockHeight == 0 {
-		return 0
-	}
-	// we project how long it has been in milliseconds since the last Ethereum block height was observed
-	projectedMillis := (uint64(currentFxHeight) - heights.BlockHeight) * params.AverageBlockTime
-	// we convert that projection into the current Ethereum height using the average Ethereum block time in millis
-	projectedCurrentEthereumHeight := (projectedMillis / params.AverageExternalBlockTime) + heights.ExternalBlockHeight
-	// we convert our target time for block timeouts (lets say 12 hours) into a number of blocks to
-	// place on top of our projection of the current Ethereum block height.
-	blocksToAdd := params.ExternalBatchTimeout / params.AverageExternalBlockTime
-	return projectedCurrentEthereumHeight + blocksToAdd
-}
-
 // OutgoingTxBatchExecuted is run when the Cosmos chain detects that a batch has been executed on Ethereum
 // It frees all the transactions in the batch, then cancels all earlier batches
 func (k Keeper) OutgoingTxBatchExecuted(ctx sdk.Context, tokenContract string, batchNonce uint64) {

diff --git a/x/crosschain/keeper/bridge_call_refund.go b/x/crosschain/keeper/bridge_call_refund.go
@@ -39,10 +39,12 @@ func (k Keeper) AddRefundRecord(ctx sdk.Context, receiver string, eventNonce uin
 	snapshotOracle.EventNonces = append(snapshotOracle.EventNonces, eventNonce)
 	k.SetSnapshotOracle(ctx, snapshotOracle)
 
+	params := k.GetParams(ctx)
+	refundTimeout := k.CalExternalTimeoutHeight(ctx, params, params.BridgeCallRefundTimeout)
 	k.SetRefundRecord(ctx, &types.RefundRecord{
 		EventNonce:     eventNonce,
 		Receiver:       receiver,
-		Timeout:        k.GetBridgeCallRefundTimeout(ctx),
+		Timeout:        refundTimeout,
 		OracleSetNonce: oracleSet.Nonce,
 		Tokens:         tokens,
 	})

diff --git a/x/crosschain/keeper/grpc_query.go b/x/crosschain/keeper/grpc_query.go
@@ -365,7 +365,9 @@ func (k Keeper) Oracles(c context.Context, _ *types.QueryOraclesRequest) (*types
 }
 
 func (k Keeper) ProjectedBatchTimeoutHeight(c context.Context, _ *types.QueryProjectedBatchTimeoutHeightRequest) (*types.QueryProjectedBatchTimeoutHeightResponse, error) {
-	timeout := k.GetBatchTimeoutHeight(sdk.UnwrapSDKContext(c))
+	ctx := sdk.UnwrapSDKContext(c)
+	params := k.GetParams(ctx)
+	timeout := k.CalExternalTimeoutHeight(ctx, params, params.ExternalBatchTimeout)
 	return &types.QueryProjectedBatchTimeoutHeightResponse{TimeoutHeight: timeout}, nil
 }
 

diff --git a/x/crosschain/types/params.go b/x/crosschain/types/params.go
@@ -3,7 +3,6 @@ package types
 import (
 	"errors"
 	"fmt"
-	"time"
 
 	sdkmath "cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -18,7 +17,7 @@ const (
 	MaxResults                     = 100
 	MaxOracleSetRequestsResults    = 5
 	MaxKeepEventSize               = 100
-	DefaultBridgeCallRefundTimeout = uint64(7 * 24 * time.Hour)
+	DefaultBridgeCallRefundTimeout = 604_800_000 // 7 * 24 * 3600 * 1000
 )
 
 var (
@@ -98,7 +97,7 @@ func (m *Params) ValidateBasic() error {
 	if len(m.Oracles) > 0 {
 		return errors.New("deprecated oracles")
 	}
-	if m.BridgeCallRefundTimeout <= uint64(1*time.Hour) {
+	if m.BridgeCallRefundTimeout <= 3_600_000 {
 		return fmt.Errorf("invalid bridge call refund timeout")
 	}
 	return nil