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.

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

CCIP-3862 Add tokenDestGasOverhead to CalculateMessageMaxGas #14964

Merged
merged 1 commit into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/giant-cats-run.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#internal Add tokenDestGasOverhead to CalculateMessageMaxGas
28 changes: 24 additions & 4 deletions core/capabilities/ccip/ccipevm/gas_helpers.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ccipevm

import (
"fmt"
"math"

cciptypes "github.com/smartcontractkit/chainlink-ccip/pkg/types/ccipocr3"
Expand Down Expand Up @@ -52,15 +53,33 @@ func bytesForMsgTokens(numTokens int) int {

// CalculateMessageMaxGas computes the maximum gas overhead for a message.
func (gp EstimateProvider) CalculateMessageMaxGas(msg cciptypes.Message) uint64 {
maxGas, err := gp.CalculateMessageMaxGasWithError(msg)
if err != nil {
panic(err)
}
return maxGas
}

// CalculateMessageMaxGasWithError computes the maximum gas overhead for a message.
// TODO: Add destGasOverhead, see:
// https://github.com/smartcontractkit/chainlink/blob/23452266132228234312947660374fb393e96f1a/contracts/src/v0.8/ccip/FeeQuoter.sol#L97
func (gp EstimateProvider) CalculateMessageMaxGasWithError(msg cciptypes.Message) (uint64, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

CalculateMessageMaxGasWithError naming implies to me that we are doing something useful with the error. Panic'ing is ...useful, just not the usual application. I see both API's are public and exportable - is there a plan to deprecate CalculateMessageMaxGas

numTokens := len(msg.TokenAmounts)
var data []byte = msg.Data
dataLength := len(data)

// TODO: update interface to return error?
// Although this decoding should never fail.
messageGasLimit, err := decodeExtraArgsV1V2(msg.ExtraArgs)
if err != nil {
panic(err)
return 0, fmt.Errorf("failed to decode extra args: %w", err)
}

var totalTokenDestGasOverhead uint64
for _, rampTokenAmount := range msg.TokenAmounts {
tokenDestGasOverhead, err := decodeTokenDestGasOverhead(rampTokenAmount.DestExecData)
if err != nil {
return 0, fmt.Errorf("failed to decode token dest gas overhead: %w", err)
}
totalTokenDestGasOverhead += uint64(tokenDestGasOverhead)
}

messageBytes := ConstantMessagePartBytes +
Expand All @@ -85,5 +104,6 @@ func (gp EstimateProvider) CalculateMessageMaxGas(msg cciptypes.Message) uint64
SupportsInterfaceCheck +
rstout marked this conversation as resolved.
Show resolved Hide resolved
adminRegistryOverhead +
rateLimiterOverhead +
PerTokenOverheadGas*uint64(numTokens)
PerTokenOverheadGas*uint64(numTokens) +
totalTokenDestGasOverhead, nil
}
114 changes: 74 additions & 40 deletions core/capabilities/ccip/ccipevm/gas_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/smartcontractkit/chainlink-ccip/pkg/types/ccipocr3"
)

func Test_calculateMessageMaxGas(t *testing.T) {
type args struct {
dataLen int
numTokens int
extraArgs []byte
dataLen int
numTokens int
extraArgs []byte
tokenGasOverhead uint32
}
tests := []struct {
name string
Expand All @@ -23,41 +25,56 @@ func Test_calculateMessageMaxGas(t *testing.T) {
}{
{
name: "base",
args: args{dataLen: 5, numTokens: 2, extraArgs: makeExtraArgsV1(200_000)},
want: 1_022_264,
args: args{dataLen: 5, numTokens: 2, extraArgs: makeExtraArgsV1(200_000), tokenGasOverhead: 10},
want: 1_022_284,
},
{
name: "large",
args: args{dataLen: 1000, numTokens: 1000, extraArgs: makeExtraArgsV1(200_000)},
want: 346_677_520,
args: args{dataLen: 1000, numTokens: 1000, extraArgs: makeExtraArgsV1(200_000), tokenGasOverhead: 1},
want: 346_678_520,
},
{
name: "overheadGas test 1",
args: args{dataLen: 0, numTokens: 0, extraArgs: makeExtraArgsV1(200_000)},
args: args{dataLen: 0, numTokens: 0, extraArgs: makeExtraArgsV1(200_000), tokenGasOverhead: 100},
want: 319_920,
},
{
name: "overheadGas test 2",
args: args{dataLen: len([]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}), numTokens: 1, extraArgs: makeExtraArgsV1(200_000)},
want: 675_948,
args: args{
dataLen: len([]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}),
numTokens: 1,
extraArgs: makeExtraArgsV1(200_000),
tokenGasOverhead: 2,
},
want: 675_950,
},
{
name: "allowOOO set to true makes no difference to final gas estimate",
args: args{dataLen: 5, numTokens: 2, extraArgs: makeExtraArgsV2(200_000, true)},
want: 1_022_264,
args: args{
dataLen: 5,
numTokens: 2,
extraArgs: makeExtraArgsV2(200_000, true),
tokenGasOverhead: 100,
},
want: 1_022_464,
},
{
name: "allowOOO set to false makes no difference to final gas estimate",
args: args{dataLen: 5, numTokens: 2, extraArgs: makeExtraArgsV2(200_000, false)},
want: 1_022_264,
args: args{
dataLen: 5,
numTokens: 2,
extraArgs: makeExtraArgsV2(200_000, false),
tokenGasOverhead: 100,
},
want: 1_022_464,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
msg := ccipocr3.Message{
Data: make([]byte, tt.args.dataLen),
TokenAmounts: make([]ccipocr3.RampTokenAmount, tt.args.numTokens),
TokenAmounts: getTokenAmounts(t, tt.args.numTokens, tt.args.tokenGasOverhead),
ExtraArgs: tt.args.extraArgs,
}
ep := EstimateProvider{}
Expand All @@ -72,44 +89,48 @@ func Test_calculateMessageMaxGas(t *testing.T) {
// are combined to one function.
func TestCalculateMaxGas(t *testing.T) {
tests := []struct {
name string
numRequests int
dataLength int
numberOfTokens int
extraArgs []byte
want uint64
name string
numRequests int
dataLength int
numberOfTokens int
extraArgs []byte
tokenGasOverhead uint32
want uint64
}{
{
name: "maxGasOverheadGas 1",
numRequests: 6,
dataLength: 0,
numberOfTokens: 0,
extraArgs: makeExtraArgsV1(200_000),
want: 322_992,
name: "maxGasOverheadGas 1",
numRequests: 6,
dataLength: 0,
numberOfTokens: 0,
extraArgs: makeExtraArgsV1(200_000),
tokenGasOverhead: 10,
want: 322_992,
},
{
name: "maxGasOverheadGas 2",
numRequests: 3,
dataLength: len([]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}),
numberOfTokens: 1,
extraArgs: makeExtraArgsV1(200_000),
want: 678_508,
name: "maxGasOverheadGas 2",
numRequests: 3,
dataLength: len([]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}),
numberOfTokens: 1,
extraArgs: makeExtraArgsV1(200_000),
tokenGasOverhead: 10,
want: 678_518,
},
{
name: "v2 extra args",
numRequests: 3,
dataLength: len([]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}),
numberOfTokens: 1,
extraArgs: makeExtraArgsV2(200_000, true),
want: 678_508,
name: "v2 extra args",
numRequests: 3,
dataLength: len([]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}),
numberOfTokens: 1,
extraArgs: makeExtraArgsV2(200_000, true),
tokenGasOverhead: 10,
want: 678_518,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
msg := ccipocr3.Message{
Data: make([]byte, tt.dataLength),
TokenAmounts: make([]ccipocr3.RampTokenAmount, tt.numberOfTokens),
TokenAmounts: getTokenAmounts(t, tt.numberOfTokens, tt.tokenGasOverhead),
ExtraArgs: tt.extraArgs,
}
ep := EstimateProvider{}
Expand Down Expand Up @@ -155,3 +176,16 @@ func makeExtraArgsV2(gasLimit uint64, allowOOO bool) []byte {
extraArgs = append(extraArgs, allowOOOBytes...)
return extraArgs
}

func getTokenAmounts(t *testing.T, numTokens int, tokenGasOverhead uint32) []ccipocr3.RampTokenAmount {
tokenDestGasOverhead, err := TokenDestGasOverheadABI.Pack(tokenGasOverhead)
require.NoError(t, err)

tokenAmounts := make([]ccipocr3.RampTokenAmount, numTokens)
for i := 0; i < numTokens; i++ {
tokenAmounts[i] = ccipocr3.RampTokenAmount{
DestExecData: tokenDestGasOverhead,
}
}
return tokenAmounts
}
30 changes: 30 additions & 0 deletions core/capabilities/ccip/ccipevm/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ import (
"github.com/ethereum/go-ethereum/accounts/abi"
)

var (
abiUint32 = ABITypeOrPanic("uint32")
TokenDestGasOverheadABI = abi.Arguments{
{
Type: abiUint32,
},
}
)

func decodeExtraArgsV1V2(extraArgs []byte) (gasLimit *big.Int, err error) {
if len(extraArgs) < 4 {
return nil, fmt.Errorf("extra args too short: %d, should be at least 4 (i.e the extraArgs tag)", len(extraArgs))
Expand Down Expand Up @@ -43,3 +52,24 @@ func abiEncodeMethodInputs(abiDef abi.ABI, inputs ...interface{}) ([]byte, error
}
return packed[4:], nil // remove the method selector
}

func ABITypeOrPanic(t string) abi.Type {
abiType, err := abi.NewType(t, "", nil)
if err != nil {
panic(err)
}
return abiType
}

// Decodes the given bytes into a uint32, based on the encoding of destGasAmount in FeeQuoter.sol
func decodeTokenDestGasOverhead(destExecData []byte) (uint32, error) {
ifaces, err := TokenDestGasOverheadABI.UnpackValues(destExecData)
if err != nil {
return 0, fmt.Errorf("abi decode TokenDestGasOverheadABI: %w", err)
}
_, ok := ifaces[0].(uint32)
if !ok {
return 0, fmt.Errorf("expected uint32, got %T", ifaces[0])
}
return ifaces[0].(uint32), nil
}
Loading