Fix abi get inputs (#1023)

* add equality tests

* regenerate abis

* add extra padding tests

* override network upgrades (#839)

* override network upgrades

* genesis network upgrades defaults

* fix conflicts

* Update plugin/evm/vm.go

* update native minter

* nativeminter nit test changes

* replace fee manager packers

* remove generated abis

* Update precompile/contracts/feemanager/contract.go

Co-authored-by: Darioush Jalali <[email protected]>
Signed-off-by: Ceyhun Onur <[email protected]>

* fix comments

* readd pack/unpack tests

* add signature tests

* refactor allowlist

* add readt allowlist tests

* refactor modify allowlist functions

* add pack read allowlist output

* fix golangci

* remove unrelated signature tests

* fix reviews

* remove skip len check vars

* add native minter equality tests

* add strict mode to getInputs

* fix nits

* add get fee config output fuzz tests

* disable strict mode by default in warp

* add fee manager fuzz tests

* move tests to another test file

* move pack test to another file

* fix tests

* merge allowlist refactor

* move function name getter to type

* move signatures to unpack test

* move signatures to unpack test

* Update precompile/allowlist/allowlist.go

Co-authored-by: Darioush Jalali <[email protected]>
Signed-off-by: Ceyhun Onur <[email protected]>

* nits

* unexport test fields

* fix tests

---------

Signed-off-by: Ceyhun Onur <[email protected]>
Co-authored-by: Darioush Jalali <[email protected]>

accounts/abi/abi.go

@@ -156,12 +156,12 @@ func (abi ABI) PackOutput(name string, args ...interface{}) ([]byte, error) {
 }
 
 // getInputs gets input arguments of the given [name] method.
-func (abi ABI) getInputs(name string, data []byte) (Arguments, error) {
+func (abi ABI) getInputs(name string, data []byte, useStrictMode bool) (Arguments, error) {
 	// since there can't be naming collisions with contracts and events,
 	// we need to decide whether we're calling a method or an event
 	var args Arguments
 	if method, ok := abi.Methods[name]; ok {
-		if len(data)%32 != 0 {
+		if useStrictMode && len(data)%32 != 0 {
 			return nil, fmt.Errorf("abi: improperly formatted input: %s - Bytes: [%+v]", string(data), data)
 		}
 		args = method.Inputs
@@ -196,8 +196,8 @@ func (abi ABI) getArguments(name string, data []byte) (Arguments, error) {
 }
 
 // UnpackInput unpacks the input according to the ABI specification.
-func (abi ABI) UnpackInput(name string, data []byte) ([]interface{}, error) {
-	args, err := abi.getInputs(name, data)
+func (abi ABI) UnpackInput(name string, data []byte, useStrictMode bool) ([]interface{}, error) {
+	args, err := abi.getInputs(name, data, useStrictMode)
 	if err != nil {
 		return nil, err
 	}
@@ -216,8 +216,8 @@ func (abi ABI) Unpack(name string, data []byte) ([]interface{}, error) {
 // UnpackInputIntoInterface unpacks the input in v according to the ABI specification.
 // It performs an additional copy. Please only use, if you want to unpack into a
 // structure that does not strictly conform to the ABI structure (e.g. has additional arguments)
-func (abi ABI) UnpackInputIntoInterface(v interface{}, name string, data []byte) error {
-	args, err := abi.getInputs(name, data)
+func (abi ABI) UnpackInputIntoInterface(v interface{}, name string, data []byte, useStrictMode bool) error {
+	args, err := abi.getInputs(name, data, useStrictMode)
 	if err != nil {
 		return err
 	}

accounts/abi/abi_extra_test.go

@@ -31,18 +31,69 @@ func TestUnpackInputIntoInterface(t *testing.T) {
 		Amount: big.NewInt(100),
 		Memo:   []byte("hello"),
 	}
-	data, err := abi.Pack("receive", input.Sender, input.Amount, input.Memo)
+
+	rawData, err := abi.Pack("receive", input.Sender, input.Amount, input.Memo)
 	require.NoError(t, err)
 
-	// Unpack into interface
-	var v inputType
-	err = abi.UnpackInputIntoInterface(&v, "receive", data[4:]) // skips 4 byte selector
+	abi, err = JSON(strings.NewReader(TEST_ABI))
 	require.NoError(t, err)
 
-	// Verify unpacked values match input
-	require.Equal(t, v.Amount, input.Amount)
-	require.EqualValues(t, v.Amount, input.Amount)
-	require.True(t, bytes.Equal(v.Memo, input.Memo))
+	for _, test := range []struct {
+		name                   string
+		extraPaddingBytes      int
+		strictMode             bool
+		expectedErrorSubstring string
+	}{
+		{
+			name:       "No extra padding to input data",
+			strictMode: true,
+		},
+		{
+			name:              "Valid input data with 32 extra padding(%32) ",
+			extraPaddingBytes: 32,
+			strictMode:        true,
+		},
+		{
+			name:              "Valid input data with 64 extra padding(%32)",
+			extraPaddingBytes: 64,
+			strictMode:        true,
+		},
+		{
+			name:                   "Valid input data with extra padding indivisible by 32",
+			extraPaddingBytes:      33,
+			strictMode:             true,
+			expectedErrorSubstring: "abi: improperly formatted input:",
+		},
+		{
+			name:              "Valid input data with extra padding indivisible by 32, no strict mode",
+			extraPaddingBytes: 33,
+			strictMode:        false,
+		},
+	} {
+		{
+			t.Run(test.name, func(t *testing.T) {
+				// skip 4 byte selector
+				data := rawData[4:]
+				// Add extra padding to data
+				data = append(data, make([]byte, test.extraPaddingBytes)...)
+
+				// Unpack into interface
+				var v inputType
+				err = abi.UnpackInputIntoInterface(&v, "receive", data, test.strictMode) // skips 4 byte selector
+
+				if test.expectedErrorSubstring != "" {
+					require.Error(t, err)
+					require.ErrorContains(t, err, test.expectedErrorSubstring)
+				} else {
+					require.NoError(t, err)
+					// Verify unpacked values match input
+					require.Equal(t, v.Amount, input.Amount)
+					require.EqualValues(t, v.Amount, input.Amount)
+					require.True(t, bytes.Equal(v.Memo, input.Memo))
+				}
+			})
+		}
+	}
 }
 
 func TestPackOutput(t *testing.T) {

accounts/abi/bind/precompilebind/precompile_contract_template.go

@@ -143,7 +143,8 @@ func Set{{.Contract.Type}}AllowListStatus(stateDB contract.StateDB, address comm
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
 func Unpack{{capitalise .Normalized.Name}}Input(input []byte) ({{capitalise .Normalized.Name}}Input, error) {
 	inputStruct := {{capitalise .Normalized.Name}}Input{}
-	err := {{$contract.Type}}ABI.UnpackInputIntoInterface(&inputStruct, "{{.Original.Name}}", input)
+	// The strict mode in decoding is disabled after DUpgrade. You can re-enable by changing the last argument to true.
+	err := {{$contract.Type}}ABI.UnpackInputIntoInterface(&inputStruct, "{{.Original.Name}}", input, false)
 
 	return inputStruct, err
 }
@@ -159,7 +160,8 @@ func Pack{{.Normalized.Name}}(inputStruct {{capitalise .Normalized.Name}}Input)
 // Unpack{{capitalise .Normalized.Name}}Input attempts to unpack [input] into the {{$bindedType}} type argument
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
 func Unpack{{capitalise .Normalized.Name}}Input(input []byte)({{$bindedType}}, error) {
-res, err := {{$contract.Type}}ABI.UnpackInput("{{$method.Original.Name}}", input)
+// The strict mode in decoding is disabled after DUpgrade. You can re-enable by changing the last argument to true.
+res, err := {{$contract.Type}}ABI.UnpackInput("{{$method.Original.Name}}", input, false)
 if err != nil {
 	return {{bindtypenew $input.Type $structs}}, err
 }

precompile/allowlist/allowlist.go

@@ -63,14 +63,14 @@ func PackModifyAllowList(address common.Address, role Role) ([]byte, error) {
 	return AllowListABI.Pack(funcName, address)
 }
 
-func UnpackModifyAllowListInput(input []byte, r Role, skipLenCheck bool) (common.Address, error) {
-	if !skipLenCheck && len(input) != allowListInputLen {
+func UnpackModifyAllowListInput(input []byte, r Role, useStrictMode bool) (common.Address, error) {
+	if useStrictMode && len(input) != allowListInputLen {
 		return common.Address{}, fmt.Errorf("invalid input length for modifying allow list: %d", len(input))
 	}
 
 	funcName := r.GetSetterFunctionName()
 	var modifyAddress common.Address
-	err := AllowListABI.UnpackInputIntoInterface(&modifyAddress, funcName, input)
+	err := AllowListABI.UnpackInputIntoInterface(&modifyAddress, funcName, input, useStrictMode)
 	return modifyAddress, err
 }
 
@@ -82,8 +82,9 @@ func createAllowListRoleSetter(precompileAddr common.Address, role Role) contrac
 			return nil, 0, err
 		}
 
-		// We skip the fixed length check with DUpgrade
-		modifyAddress, err := UnpackModifyAllowListInput(input, role, contract.IsDUpgradeActivated(evm))
+		// do not use strict mode after DUpgrade
+		useStrictMode := !contract.IsDUpgradeActivated(evm)
+		modifyAddress, err := UnpackModifyAllowListInput(input, role, useStrictMode)
 
 		if err != nil {
 			return nil, remainingGas, err
@@ -113,13 +114,13 @@ func PackReadAllowList(address common.Address) ([]byte, error) {
 	return AllowListABI.Pack("readAllowList", address)
 }
 
-func UnpackReadAllowListInput(input []byte, skipLenCheck bool) (common.Address, error) {
-	if !skipLenCheck && len(input) != allowListInputLen {
+func UnpackReadAllowListInput(input []byte, useStrictMode bool) (common.Address, error) {
+	if useStrictMode && len(input) != allowListInputLen {
 		return common.Address{}, fmt.Errorf("invalid input length for read allow list: %d", len(input))
 	}
 
 	var modifyAddress common.Address
-	err := AllowListABI.UnpackInputIntoInterface(&modifyAddress, "readAllowList", input)
+	err := AllowListABI.UnpackInputIntoInterface(&modifyAddress, "readAllowList", input, useStrictMode)
 	return modifyAddress, err
 }
 
@@ -137,7 +138,8 @@ func createReadAllowList(precompileAddr common.Address) contract.RunStatefulPrec
 		}
 
 		// We skip the fixed length check with DUpgrade
-		readAddress, err := UnpackReadAllowListInput(input, contract.IsDUpgradeActivated(evm))
+		useStrictMode := !contract.IsDUpgradeActivated(evm)
+		readAddress, err := UnpackReadAllowListInput(input, useStrictMode)
 		if err != nil {
 			return nil, remainingGas, err
 		}

precompile/allowlist/unpack_pack_test.go

@@ -53,7 +53,7 @@ func FuzzPackReadAllowlistTest(f *testing.F) {
 func FuzzPackReadAllowlistTestSkipCheck(f *testing.F) {
 	f.Fuzz(func(t *testing.T, b []byte) {
 		require := require.New(t)
-		res, err := UnpackReadAllowListInput(b, false)
+		res, err := UnpackReadAllowListInput(b, true)
 		oldRes, oldErr := OldUnpackReadAllowList(b)
 		if oldErr != nil {
 			require.ErrorContains(err, oldErr.Error())
@@ -77,7 +77,7 @@ func testPackReadAllowlistTest(t *testing.T, address common.Address) {
 		require.NoError(err)
 		// exclude 4 bytes for function selector
 		input = input[4:]
-		unpacked, err := UnpackReadAllowListInput(input, false)
+		unpacked, err := UnpackReadAllowListInput(input, true)
 		require.NoError(err)
 		require.Equal(address, unpacked)
 
@@ -99,7 +99,7 @@ func testPackReadAllowlistTest(t *testing.T, address common.Address) {
 		// exclude 4 bytes for function selector
 		input2 = input2[4:]
 		require.Equal(input, input2)
-		unpacked, err = UnpackReadAllowListInput(input2, false)
+		unpacked, err = UnpackReadAllowListInput(input2, true)
 		require.NoError(err)
 		unpacked2, err := OldUnpackReadAllowList(input)
 		require.NoError(err)
@@ -121,7 +121,7 @@ func FuzzPackModifyAllowListTest(f *testing.F) {
 func FuzzPackModifyAllowlistTestSkipCheck(f *testing.F) {
 	f.Fuzz(func(t *testing.T, b []byte) {
 		require := require.New(t)
-		res, err := UnpackModifyAllowListInput(b, AdminRole, false)
+		res, err := UnpackModifyAllowListInput(b, AdminRole, true)
 		oldRes, oldErr := OldUnpackModifyAllowList(b, AdminRole)
 		if oldErr != nil {
 			require.ErrorContains(err, oldErr.Error())
@@ -141,7 +141,7 @@ func testPackModifyAllowListTest(t *testing.T, address common.Address, role Role
 		require.NoError(err)
 		// exclude 4 bytes for function selector
 		input = input[4:]
-		unpacked, err := UnpackModifyAllowListInput(input, role, false)
+		unpacked, err := UnpackModifyAllowListInput(input, role, true)
 		require.NoError(err)
 		require.Equal(address, unpacked)
 
@@ -167,7 +167,7 @@ func testPackModifyAllowListTest(t *testing.T, address common.Address, role Role
 		// exclude 4 bytes for function selector
 		input2 = input2[4:]
 		require.Equal(input, input2)
-		unpacked, err = UnpackModifyAllowListInput(input2, role, false)
+		unpacked, err = UnpackModifyAllowListInput(input2, role, true)
 		require.NoError(err)
 		unpacked2, err := OldUnpackModifyAllowList(input, role)
 		require.NoError(err)

precompile/contracts/feemanager/contract.go

@@ -173,17 +173,17 @@ func PackSetFeeConfig(input commontype.FeeConfig) ([]byte, error) {
 
 // UnpackSetFeeConfigInput attempts to unpack [input] as SetFeeConfigInput
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
-// if [skipLenCheck] is false, it will return an error if the length of [input] is not [feeConfigInputLen]
-func UnpackSetFeeConfigInput(input []byte, skipLenCheck bool) (commontype.FeeConfig, error) {
+// if [useStrictMode] is true, it will return an error if the length of [input] is not [feeConfigInputLen]
+func UnpackSetFeeConfigInput(input []byte, useStrictMode bool) (commontype.FeeConfig, error) {
 	// Initially we had this check to ensure that the input was the correct length.
 	// However solidity does not always pack the input to the correct length, and allows
 	// for extra padding bytes to be added to the end of the input. Therefore, we have removed
 	// this check with the DUpgrade. We still need to keep this check for backwards compatibility.
-	if !skipLenCheck && len(input) != feeConfigInputLen {
+	if useStrictMode && len(input) != feeConfigInputLen {
 		return commontype.FeeConfig{}, fmt.Errorf("%w: %d", ErrInvalidLen, len(input))
 	}
 	inputStruct := FeeConfigABIStruct{}
-	err := FeeManagerABI.UnpackInputIntoInterface(&inputStruct, "setFeeConfig", input)
+	err := FeeManagerABI.UnpackInputIntoInterface(&inputStruct, "setFeeConfig", input, useStrictMode)
 	if err != nil {
 		return commontype.FeeConfig{}, err
 	}
@@ -213,8 +213,9 @@ func setFeeConfig(accessibleState contract.AccessibleState, caller common.Addres
 		return nil, remainingGas, vmerrs.ErrWriteProtection
 	}
 
-	// We skip the fixed length check with DUpgrade
-	feeConfig, err := UnpackSetFeeConfigInput(input, contract.IsDUpgradeActivated(accessibleState))
+	// do not use strict mode after DUpgrade
+	useStrictMode := !contract.IsDUpgradeActivated(accessibleState)
+	feeConfig, err := UnpackSetFeeConfigInput(input, useStrictMode)
 	if err != nil {
 		return nil, remainingGas, err
 	}

precompile/contracts/feemanager/contract_test.go

@@ -20,6 +20,17 @@ import (
 )
 
 var (
+	regressionBytes     = "8f10b58600000000000000000000000000000000000000000000000000000000017d78400000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000012a05f20000000000000000000000000000000000000000000000000000000000047868c0000000000000000000000000000000000000000000000000000000000000005400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001bc16d674ec800000000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000"
+	regressionFeeConfig = commontype.FeeConfig{
+		GasLimit:                 big.NewInt(25000000),
+		TargetBlockRate:          2,
+		MinBaseFee:               big.NewInt(5000000000),
+		TargetGas:                big.NewInt(75000000),
+		BaseFeeChangeDenominator: big.NewInt(84),
+		MinBlockGasCost:          big.NewInt(0),
+		MaxBlockGasCost:          big.NewInt(2000000000000000000),
+		BlockGasCostStep:         big.NewInt(1000000000000000000),
+	}
 	testFeeConfig = commontype.FeeConfig{
 		GasLimit:        big.NewInt(8_000_000),
 		TargetBlockRate: 2, // in seconds
@@ -323,6 +334,47 @@ var (
 				require.EqualValues(t, testBlockNumber, lastChangedAt)
 			},
 		},
+		// from https://github.com/ava-labs/subnet-evm/issues/487
+		"setFeeConfig regression test should fail before DUpgrade": {
+			Caller:     allowlist.TestEnabledAddr,
+			BeforeHook: allowlist.SetDefaultRoles(Module.Address),
+			Input:      common.Hex2Bytes(regressionBytes),
+			ChainConfigFn: func(t testing.TB) precompileconfig.ChainConfig {
+				config := precompileconfig.NewMockChainConfig(gomock.NewController(t))
+				config.EXPECT().IsDUpgrade(gomock.Any()).Return(false).AnyTimes()
+				return config
+			},
+			SuppliedGas: SetFeeConfigGasCost,
+			ExpectedErr: ErrInvalidLen.Error(),
+			ReadOnly:    false,
+			SetupBlockContext: func(mbc *contract.MockBlockContext) {
+				mbc.EXPECT().Number().Return(testBlockNumber).AnyTimes()
+				mbc.EXPECT().Timestamp().Return(uint64(0)).AnyTimes()
+			},
+		},
+		"setFeeConfig regression test should succeed after DUpgrade": {
+			Caller:     allowlist.TestEnabledAddr,
+			BeforeHook: allowlist.SetDefaultRoles(Module.Address),
+			Input:      common.Hex2Bytes(regressionBytes),
+			ChainConfigFn: func(t testing.TB) precompileconfig.ChainConfig {
+				config := precompileconfig.NewMockChainConfig(gomock.NewController(t))
+				config.EXPECT().IsDUpgrade(gomock.Any()).Return(true).AnyTimes()
+				return config
+			},
+			SuppliedGas: SetFeeConfigGasCost,
+			ReadOnly:    false,
+			ExpectedRes: []byte{},
+			SetupBlockContext: func(mbc *contract.MockBlockContext) {
+				mbc.EXPECT().Number().Return(testBlockNumber).AnyTimes()
+				mbc.EXPECT().Timestamp().Return(uint64(0)).AnyTimes()
+			},
+			AfterHook: func(t testing.TB, state contract.StateDB) {
+				feeConfig := GetStoredFeeConfig(state)
+				require.Equal(t, regressionFeeConfig, feeConfig)
+				lastChangedAt := GetFeeConfigLastChangedAt(state)
+				require.EqualValues(t, testBlockNumber, lastChangedAt)
+			},
+		},
 	}
 )
 

precompile/contracts/feemanager/unpack_pack_test.go

@@ -233,58 +233,72 @@ func TestPackSetFeeConfigInput(t *testing.T) {
 	tests := []struct {
 		name           string
 		input          []byte
-		skipLenCheck   bool
+		strictMode     bool
 		expectedErr    string
 		expectedOldErr string
 		expectedOutput commontype.FeeConfig
 	}{
 		{
-			name:           "empty input",
+			name:           "empty input strict mode",
 			input:          []byte{},
-			skipLenCheck:   false,
+			strictMode:     true,
 			expectedErr:    ErrInvalidLen.Error(),
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
 		{
-			name:           "empty input skip len check",
+			name:           "empty input",
 			input:          []byte{},
-			skipLenCheck:   true,
+			strictMode:     false,
 			expectedErr:    "attempting to unmarshall an empty string",
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
+		{
+			name:           "input with insufficient len strict mode",
+			input:          []byte{123},
+			strictMode:     true,
+			expectedErr:    ErrInvalidLen.Error(),
+			expectedOldErr: ErrInvalidLen.Error(),
+		},
+		{
+			name:           "input with insufficient len",
+			input:          []byte{123},
+			strictMode:     false,
+			expectedErr:    "length insufficient",
+			expectedOldErr: ErrInvalidLen.Error(),
+		},
+		{
+			name:           "input with extra bytes strict mode",
+			input:          append(testInputBytes, make([]byte, 32)...),
+			strictMode:     true,
+			expectedErr:    ErrInvalidLen.Error(),
+			expectedOldErr: ErrInvalidLen.Error(),
+		},
 		{
 			name:           "input with extra bytes",
 			input:          append(testInputBytes, make([]byte, 32)...),
-			skipLenCheck:   false,
-			expectedErr:    ErrInvalidLen.Error(),
-			expectedOldErr: ErrInvalidLen.Error(),
-		},
-		{
-			name:           "input with extra bytes skip len check",
-			input:          append(testInputBytes, make([]byte, 32)...),
-			skipLenCheck:   true,
+			strictMode:     false,
 			expectedErr:    "",
 			expectedOldErr: ErrInvalidLen.Error(),
 			expectedOutput: testFeeConfig,
 		},
 		{
-			name:           "input with extra bytes (not divisible by 32)",
+			name:           "input with extra bytes (not divisible by 32) strict mode",
 			input:          append(testInputBytes, make([]byte, 33)...),
-			skipLenCheck:   false,
+			strictMode:     true,
 			expectedErr:    ErrInvalidLen.Error(),
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
 		{
-			name:           "input with extra bytes (not divisible by 32) skip len check",
+			name:           "input with extra bytes (not divisible by 32)",
 			input:          append(testInputBytes, make([]byte, 33)...),
-			skipLenCheck:   true,
-			expectedErr:    "improperly formatted input",
+			strictMode:     false,
+			expectedOutput: testFeeConfig,
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			unpacked, err := UnpackSetFeeConfigInput(test.input, test.skipLenCheck)
+			unpacked, err := UnpackSetFeeConfigInput(test.input, test.strictMode)
 			if test.expectedErr != "" {
 				require.ErrorContains(t, err, test.expectedErr)
 			} else {
@@ -374,7 +388,7 @@ func testOldPackSetFeeConfigInputEqual(t *testing.T, feeConfig commontype.FeeCon
 		require.Equal(t, input, input2)
 
 		value, err := OldUnpackFeeConfig(input)
-		unpacked, err2 := UnpackSetFeeConfigInput(input, false)
+		unpacked, err2 := UnpackSetFeeConfigInput(input, true)
 		if err != nil {
 			require.ErrorContains(t, err2, err.Error())
 			return

precompile/contracts/nativeminter/contract.go

@@ -58,17 +58,17 @@ func PackMintNativeCoin(address common.Address, amount *big.Int) ([]byte, error)
 
 // UnpackMintNativeCoinInput attempts to unpack [input] as address and amount.
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
-// if [skipLenCheck] is false, it will return an error if the length of [input] is not [mintInputLen]
-func UnpackMintNativeCoinInput(input []byte, skipLenCheck bool) (common.Address, *big.Int, error) {
+// if [useStrictMode] is true, it will return an error if the length of [input] is not [mintInputLen]
+func UnpackMintNativeCoinInput(input []byte, useStrictMode bool) (common.Address, *big.Int, error) {
 	// Initially we had this check to ensure that the input was the correct length.
 	// However solidity does not always pack the input to the correct length, and allows
 	// for extra padding bytes to be added to the end of the input. Therefore, we have removed
 	// this check with the DUpgrade. We still need to keep this check for backwards compatibility.
-	if !skipLenCheck && len(input) != mintInputLen {
+	if useStrictMode && len(input) != mintInputLen {
 		return common.Address{}, nil, fmt.Errorf("%w: %d", ErrInvalidLen, len(input))
 	}
 	inputStruct := MintNativeCoinInput{}
-	err := NativeMinterABI.UnpackInputIntoInterface(&inputStruct, "mintNativeCoin", input)
+	err := NativeMinterABI.UnpackInputIntoInterface(&inputStruct, "mintNativeCoin", input, useStrictMode)
 
 	return inputStruct.Addr, inputStruct.Amount, err
 }
@@ -84,8 +84,8 @@ func mintNativeCoin(accessibleState contract.AccessibleState, caller common.Addr
 		return nil, remainingGas, vmerrs.ErrWriteProtection
 	}
 
-	// We skip the fixed length check with DUpgrade
-	to, amount, err := UnpackMintNativeCoinInput(input, contract.IsDUpgradeActivated(accessibleState))
+	useStrictMode := !contract.IsDUpgradeActivated(accessibleState)
+	to, amount, err := UnpackMintNativeCoinInput(input, useStrictMode)
 	if err != nil {
 		return nil, remainingGas, err
 	}

precompile/contracts/nativeminter/unpack_pack_test.go

@@ -52,60 +52,61 @@ func TestUnpackMintNativeCoinInput(t *testing.T) {
 	tests := []struct {
 		name           string
 		input          []byte
-		skipLenCheck   bool
+		strictMode     bool
 		expectedErr    string
 		expectedOldErr string
 		expectedAddr   common.Address
 		expectedAmount *big.Int
 	}{
 		{
-			name:           "empty input",
+			name:           "empty input strict mode",
 			input:          []byte{},
-			skipLenCheck:   false,
+			strictMode:     true,
 			expectedErr:    ErrInvalidLen.Error(),
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
 		{
-			name:           "empty input skip len check",
+			name:           "empty input",
 			input:          []byte{},
-			skipLenCheck:   true,
+			strictMode:     false,
 			expectedErr:    "attempting to unmarshall an empty string",
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
+		{
+			name:           "input with extra bytes strict mode",
+			input:          append(testInputBytes, make([]byte, 32)...),
+			strictMode:     true,
+			expectedErr:    ErrInvalidLen.Error(),
+			expectedOldErr: ErrInvalidLen.Error(),
+		},
 		{
 			name:           "input with extra bytes",
 			input:          append(testInputBytes, make([]byte, 32)...),
-			skipLenCheck:   false,
-			expectedErr:    ErrInvalidLen.Error(),
-			expectedOldErr: ErrInvalidLen.Error(),
-		},
-		{
-			name:           "input with extra bytes skip len check",
-			input:          append(testInputBytes, make([]byte, 32)...),
-			skipLenCheck:   true,
+			strictMode:     false,
 			expectedErr:    "",
 			expectedOldErr: ErrInvalidLen.Error(),
 			expectedAddr:   constants.BlackholeAddr,
 			expectedAmount: common.Big2,
 		},
 		{
-			name:           "input with extra bytes (not divisible by 32)",
+			name:           "input with extra bytes (not divisible by 32) strict mode",
 			input:          append(testInputBytes, make([]byte, 33)...),
-			skipLenCheck:   false,
+			strictMode:     true,
 			expectedErr:    ErrInvalidLen.Error(),
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
 		{
-			name:           "input with extra bytes (not divisible by 32) skip len check",
+			name:           "input with extra bytes (not divisible by 32)",
 			input:          append(testInputBytes, make([]byte, 33)...),
-			skipLenCheck:   true,
-			expectedErr:    "improperly formatted input",
+			strictMode:     false,
+			expectedAddr:   constants.BlackholeAddr,
+			expectedAmount: common.Big2,
 			expectedOldErr: ErrInvalidLen.Error(),
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			unpackedAddress, unpackedAmount, err := UnpackMintNativeCoinInput(test.input, test.skipLenCheck)
+			unpackedAddress, unpackedAmount, err := UnpackMintNativeCoinInput(test.input, test.strictMode)
 			if test.expectedErr != "" {
 				require.ErrorContains(t, err, test.expectedErr)
 			} else {
@@ -145,7 +146,7 @@ func testOldPackMintNativeCoinEqual(t *testing.T, addr common.Address, amount *b
 
 		input = input[4:]
 		to, assetAmount, err := OldUnpackMintNativeCoinInput(input)
-		unpackedAddr, unpackedAmount, err2 := UnpackMintNativeCoinInput(input, false)
+		unpackedAddr, unpackedAmount, err2 := UnpackMintNativeCoinInput(input, true)
 		if err != nil {
 			require.ErrorContains(t, err2, err.Error())
 			return

precompile/contracts/rewardmanager/contract.go

@@ -174,8 +174,9 @@ func PackSetRewardAddress(addr common.Address) ([]byte, error) {
 
 // UnpackSetRewardAddressInput attempts to unpack [input] into the common.Address type argument
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
-func UnpackSetRewardAddressInput(input []byte) (common.Address, error) {
-	res, err := RewardManagerABI.UnpackInput("setRewardAddress", input)
+// if [useStrictMode] is true, it will return an error if the length of [input] is not divisible by 32
+func UnpackSetRewardAddressInput(input []byte, useStrictMode bool) (common.Address, error) {
+	res, err := RewardManagerABI.UnpackInput("setRewardAddress", input, useStrictMode)
 	if err != nil {
 		return common.Address{}, err
 	}
@@ -192,8 +193,9 @@ func setRewardAddress(accessibleState contract.AccessibleState, caller common.Ad
 	}
 	// attempts to unpack [input] into the arguments to the SetRewardAddressInput.
 	// Assumes that [input] does not include selector
-	// You can use unpacked [inputStruct] variable in your code
-	inputStruct, err := UnpackSetRewardAddressInput(input)
+	// do not use strict mode after DUpgrade
+	useStrictMode := !contract.IsDUpgradeActivated(accessibleState)
+	inputStruct, err := UnpackSetRewardAddressInput(input, useStrictMode)
 	if err != nil {
 		return nil, remainingGas, err
 	}

x/warp/contract.go

@@ -109,7 +109,9 @@ func getBlockchainID(accessibleState contract.AccessibleState, caller common.Add
 // UnpackGetVerifiedWarpBlockHashInput attempts to unpack [input] into the uint32 type argument
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
 func UnpackGetVerifiedWarpBlockHashInput(input []byte) (uint32, error) {
-	res, err := WarpABI.UnpackInput("getVerifiedWarpBlockHash", input)
+	// We don't use strict mode here because it was disabled with the DUpgrade.
+	// Since Warp will be deployed after the DUpgrade, we don't need to use strict mode
+	res, err := WarpABI.UnpackInput("getVerifiedWarpBlockHash", input, false)
 	if err != nil {
 		return 0, err
 	}
@@ -149,7 +151,9 @@ func getVerifiedWarpBlockHash(accessibleState contract.AccessibleState, caller c
 // UnpackGetVerifiedWarpMessageInput attempts to unpack [input] into the uint32 type argument
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
 func UnpackGetVerifiedWarpMessageInput(input []byte) (uint32, error) {
-	res, err := WarpABI.UnpackInput("getVerifiedWarpMessage", input)
+	// We don't use strict mode here because it was disabled with the DUpgrade.
+	// Since Warp will be deployed after the DUpgrade, we don't need to use strict mode.
+	res, err := WarpABI.UnpackInput("getVerifiedWarpMessage", input, false)
 	if err != nil {
 		return 0, err
 	}
@@ -191,7 +195,9 @@ func getVerifiedWarpMessage(accessibleState contract.AccessibleState, caller com
 // UnpackSendWarpMessageInput attempts to unpack [input] as []byte
 // assumes that [input] does not include selector (omits first 4 func signature bytes)
 func UnpackSendWarpMessageInput(input []byte) ([]byte, error) {
-	res, err := WarpABI.UnpackInput("sendWarpMessage", input)
+	// We don't use strict mode here because it was disabled with the DUpgrade.
+	// Since Warp will be deployed after the DUpgrade, we don't need to use strict mode.
+	res, err := WarpABI.UnpackInput("sendWarpMessage", input, false)
 	if err != nil {
 		return []byte{}, err
 	}