From 2920035fad7bf7b5cd75b6cc1d5d2253297375e5 Mon Sep 17 00:00:00 2001 From: Iulian Pascalau Date: Mon, 26 Feb 2024 13:40:26 +0200 Subject: [PATCH] - minor refactor for the ConvertToDisplayableData function --- bridges/ethMultiversX/bridgeExecutor.go | 126 +++++++++++------- bridges/ethMultiversX/bridgeExecutor_test.go | 33 ++--- clients/multiversx/client.go | 2 +- clients/multiversx/client_test.go | 12 +- clients/multiversx/mxClientDataGetter_test.go | 12 +- .../relayers/ethToMultiversX_test.go | 12 +- 6 files changed, 105 insertions(+), 92 deletions(-) diff --git a/bridges/ethMultiversX/bridgeExecutor.go b/bridges/ethMultiversX/bridgeExecutor.go index a689240b..85bb3236 100644 --- a/bridges/ethMultiversX/bridgeExecutor.go +++ b/bridges/ethMultiversX/bridgeExecutor.go @@ -22,9 +22,14 @@ import ( // we wait for the transfer confirmation on Ethereum const splits = 10 const minRetries = 1 +const uint32ArgBytes = 4 +const uint64ArgBytes = 8 -// MissingCallData is a placeholder that specifies that the call data is missing -var MissingCallData = []byte{0x00} +// MissingDataProtocolMarker defines the marker for missing data (simple transfers) +const MissingDataProtocolMarker byte = 0x00 + +// DataPresentProtocolMarker defines the marker for existing data (transfers with SC calls) +const DataPresentProtocolMarker byte = 0x01 // ArgsBridgeExecutor is the arguments DTO struct used in both bridges type ArgsBridgeExecutor struct { @@ -496,7 +501,7 @@ func (executor *bridgeExecutor) addMetadataToTransfer(transfer *clients.DepositT } } - transfer.Data = MissingCallData + transfer.Data = []byte{MissingDataProtocolMarker} transfer.DisplayableData = "" return transfer @@ -721,72 +726,95 @@ func ConvertToDisplayableData(callData []byte) (string, error) { return "", fmt.Errorf("callData too short for protocol indicator") } - if callData[0] == 0x00 { + marker := callData[0] + callData = callData[1:] + + switch marker { + case MissingDataProtocolMarker: return "", nil + case DataPresentProtocolMarker: + return convertBytesToDisplayableData(callData) + default: + return "", fmt.Errorf("callData unexpected protocol indicator: %d", marker) } +} - if callData[0] != 0x01 { - return "", fmt.Errorf("callData unexpected protocol indicator: %d", callData[0]) +func convertBytesToDisplayableData(callData []byte) (string, error) { + callData, endpointName, err := extractString(callData) + if err != nil { + return "", fmt.Errorf("%w for endpoint", err) } - currentIndex := uint64(1) - // Ensure there's enough length for the initial parts: 1 byte protocol indicator + 4 bytes for endpoint name length - if len(callData) < int(currentIndex+4) { - return "", fmt.Errorf("callData too short for endpoint name length") + callData, gasLimit, err := extractGasLimit(callData) + if err != nil { + return "", err } - endpointNameLength := uint64(binary.BigEndian.Uint32(callData[currentIndex : currentIndex+4])) - currentIndex += 4 - // Check if there's enough length for the endpoint name itself - if len(callData) < int(currentIndex+endpointNameLength) { - return "", fmt.Errorf("callData too short for endpoint name") + callData, numArgumentsLength, err := extractArgumentsLen(callData) + if err != nil { + return "", err } - endpointName := string(callData[currentIndex : currentIndex+endpointNameLength]) - currentIndex += endpointNameLength - // Check for gas limit - if len(callData) < int(currentIndex+8) { // 8 bytes for gas limit length - return "", fmt.Errorf("callData too short for gas limit length") + arguments := make([]string, 0) + for i := 0; i < numArgumentsLength; i++ { + var argument string + callData, argument, err = extractString(callData) + if err != nil { + return "", fmt.Errorf("%w for argument %d", err, i) + } + + arguments = append(arguments, argument) } - gasLimitLength := binary.BigEndian.Uint64(callData[currentIndex : currentIndex+8]) - currentIndex += 8 + return fmt.Sprintf("Endpoint: %s, Gas: %d, Arguments: %s", endpointName, gasLimit, strings.Join(arguments, "@")), nil +} - // Check for gas limit - if len(callData) < int(currentIndex+gasLimitLength) { - return "", fmt.Errorf("callData too short for gas limit") +func extractString(callData []byte) ([]byte, string, error) { + // Ensure there's enough length for the 4 bytes for length + if len(callData) < uint32ArgBytes { + return nil, "", fmt.Errorf("callData too short while extracting the length") } + argumentLength := int(binary.BigEndian.Uint32(callData[:uint32ArgBytes])) + callData = callData[uint32ArgBytes:] // remove the len bytes - gasLimitBytes := append(bytes.Repeat([]byte{0x00}, 8-int(gasLimitLength)), callData[currentIndex:currentIndex+gasLimitLength]...) - gasLimit := binary.BigEndian.Uint64(gasLimitBytes) - currentIndex += gasLimitLength + // Check for the argument data + if len(callData) < argumentLength { + return nil, "", fmt.Errorf("callData too short while extracting the string data") + } + endpointName := string(callData[:argumentLength]) + callData = callData[argumentLength:] // remove the string bytes + + return callData, endpointName, nil +} - // Check for numArguments - if len(callData) < int(currentIndex+4) { // 4 for numArguments - return "", fmt.Errorf("callData too short for numArguments length") +func extractGasLimit(callData []byte) ([]byte, uint64, error) { + // Check for gas limit + if len(callData) < uint64ArgBytes { // 8 bytes for gas limit length + return nil, 0, fmt.Errorf("callData too short for gas limit length") } - numArgumentsLength := binary.BigEndian.Uint32(callData[currentIndex : currentIndex+4]) - currentIndex += 4 - arguments := make([]string, 0) + gasLimitLength := int(binary.BigEndian.Uint64(callData[:uint64ArgBytes])) + callData = callData[uint64ArgBytes:] - for i := 0; i < int(numArgumentsLength); i++ { - // Check for argument length - if len(callData) < int(currentIndex+4) { - return "", fmt.Errorf("callData too short for argument %d length", i) - } - argumentLength := uint64(binary.BigEndian.Uint32(callData[currentIndex : currentIndex+4])) - currentIndex += 4 + // Check for gas limit + if len(callData) < gasLimitLength { + return nil, 0, fmt.Errorf("callData too short for gas limit") + } - // Check for the argument data - if len(callData) < int(currentIndex+argumentLength) { - return "", fmt.Errorf("callData too short for argument %d data", i) - } - argument := callData[currentIndex : currentIndex+argumentLength] - currentIndex += argumentLength + gasLimitBytes := append(bytes.Repeat([]byte{0x00}, 8-int(gasLimitLength)), callData[:gasLimitLength]...) + gasLimit := binary.BigEndian.Uint64(gasLimitBytes) + callData = callData[gasLimitLength:] // remove the gas limit bytes - arguments = append(arguments, string(argument)) + return callData, gasLimit, nil +} + +func extractArgumentsLen(callData []byte) ([]byte, int, error) { + // Ensure there's enough length for the 4 bytes for endpoint name length + if len(callData) < uint32ArgBytes { + return nil, 0, fmt.Errorf("callData too short for numArguments length") } + length := int(binary.BigEndian.Uint32(callData[:uint32ArgBytes])) + callData = callData[uint32ArgBytes:] // remove the len bytes - return fmt.Sprintf("Endpoint: %s, Gas: %d, Arguments: %s", endpointName, gasLimit, strings.Join(arguments, "@")), nil + return callData, length, nil } diff --git a/bridges/ethMultiversX/bridgeExecutor_test.go b/bridges/ethMultiversX/bridgeExecutor_test.go index 25f22098..f3fc700c 100644 --- a/bridges/ethMultiversX/bridgeExecutor_test.go +++ b/bridges/ethMultiversX/bridgeExecutor_test.go @@ -1925,27 +1925,21 @@ func TestConvertToDisplayableData_TooShortForEndpointNameLength(t *testing.T) { t.Parallel() _, err := ConvertToDisplayableData([]byte{0x01}) require.NotNil(t, err) - if err != nil { - require.Equal(t, "callData too short for endpoint name length", err.Error()) - } + require.Equal(t, "callData too short while extracting the length for endpoint", err.Error()) } func TestConvertToDisplayableData_UnexpectedProtocolIndicator(t *testing.T) { t.Parallel() _, err := ConvertToDisplayableData([]byte{0x02}) require.NotNil(t, err) - if err != nil { - require.Equal(t, "callData unexpected protocol indicator: 2", err.Error()) - } + require.Equal(t, "callData unexpected protocol indicator: 2", err.Error()) } func TestConvertToDisplayableData_TooShortForEndpointName(t *testing.T) { t.Parallel() _, err := ConvertToDisplayableData([]byte{0x01, 0x00, 0x00, 0x00, 0x05}) require.NotNil(t, err) - if err != nil { - require.Equal(t, "callData too short for endpoint name", err.Error()) - } + require.Equal(t, "callData too short while extracting the string data for endpoint", err.Error()) } func TestConvertToDisplayableData_TooShortForGasLimitLength(t *testing.T) { @@ -1956,9 +1950,8 @@ func TestConvertToDisplayableData_TooShortForGasLimitLength(t *testing.T) { return b }() _, err := ConvertToDisplayableData(callData) - if err != nil { - require.Equal(t, "callData too short for gas limit length", err.Error()) - } + require.NotNil(t, err) + require.Equal(t, "callData too short for gas limit length", err.Error()) } func TestConvertToDisplayableData_TooShortForGasLimit(t *testing.T) { @@ -1971,9 +1964,7 @@ func TestConvertToDisplayableData_TooShortForGasLimit(t *testing.T) { }() _, err := ConvertToDisplayableData(callData) require.NotNil(t, err) - if err != nil { - require.Equal(t, "callData too short for gas limit", err.Error()) - } + require.Equal(t, "callData too short for gas limit", err.Error()) } func TestConvertToDisplayableData_TooShortForNumberOfArgumentsLength(t *testing.T) { @@ -1987,9 +1978,7 @@ func TestConvertToDisplayableData_TooShortForNumberOfArgumentsLength(t *testing. }() _, err := ConvertToDisplayableData(callData) require.NotNil(t, err) - if err != nil { - require.Equal(t, "callData too short for numArguments length", err.Error()) - } + require.Equal(t, "callData too short for numArguments length", err.Error()) } func TestConvertToDisplayableData_TooShortForArgumentLength(t *testing.T) { @@ -2004,9 +1993,7 @@ func TestConvertToDisplayableData_TooShortForArgumentLength(t *testing.T) { }() _, err := ConvertToDisplayableData(callData) require.NotNil(t, err) - if err != nil { - require.Equal(t, "callData too short for argument 0 length", err.Error()) - } + require.Equal(t, "callData too short while extracting the length for argument 0", err.Error()) } func TestConvertToDisplayableData_TooShortForArgumentData(t *testing.T) { @@ -2022,7 +2009,5 @@ func TestConvertToDisplayableData_TooShortForArgumentData(t *testing.T) { }() _, err := ConvertToDisplayableData(callData) require.NotNil(t, err) - if err != nil { - require.Equal(t, "callData too short for argument 0 data", err.Error()) - } + require.Equal(t, "callData too short while extracting the string data for argument 0", err.Error()) } diff --git a/clients/multiversx/client.go b/clients/multiversx/client.go index c71fb433..2ff9767e 100644 --- a/clients/multiversx/client.go +++ b/clients/multiversx/client.go @@ -387,7 +387,7 @@ func (c *client) PerformAction(ctx context.Context, actionID uint64, batch *clie func (c *client) computeExtraGasForSCCallsBasic(batch *clients.TransferBatch, performAction bool) uint64 { gasLimit := uint64(0) for _, deposit := range batch.Deposits { - if bytes.Equal(deposit.Data, ethmultiversx.MissingCallData) { + if bytes.Equal(deposit.Data, []byte{ethmultiversx.MissingDataProtocolMarker}) { continue } diff --git a/clients/multiversx/client_test.go b/clients/multiversx/client_test.go index b8f53e0b..6848b88a 100644 --- a/clients/multiversx/client_test.go +++ b/clients/multiversx/client_test.go @@ -583,8 +583,8 @@ func TestClient_ProposeTransfer(t *testing.T) { SendTransactionReturnHashCalled: func(ctx context.Context, builder builders.TxDataBuilder, gasLimit uint64) (string, error) { sendWasCalled = true - dataField, err := builder.ToDataString() - assert.Nil(t, err) + dataField, errConvert := builder.ToDataString() + assert.Nil(t, errConvert) dataStrings := []string{ proposeTransferFuncName, @@ -593,7 +593,7 @@ func TestClient_ProposeTransfer(t *testing.T) { extraGas := uint64(0) for _, dt := range batch.Deposits { dataStrings = append(dataStrings, depositToStrings(dt)...) - if bytes.Equal(dt.Data, ethmultiversx.MissingCallData) { + if bytes.Equal(dt.Data, []byte{ethmultiversx.MissingDataProtocolMarker}) { continue } extraGas += (uint64(len(dt.Data))*2 + 1) * args.GasMapConfig.ScCallPerByte @@ -801,8 +801,8 @@ func TestClient_PerformAction(t *testing.T) { SendTransactionReturnHashCalled: func(ctx context.Context, builder builders.TxDataBuilder, gasLimit uint64) (string, error) { sendWasCalled = true - dataField, err := builder.ToDataString() - assert.Nil(t, err) + dataField, errConvert := builder.ToDataString() + assert.Nil(t, errConvert) dataStrings := []string{ performActionFuncName, @@ -814,7 +814,7 @@ func TestClient_PerformAction(t *testing.T) { extraGas := uint64(0) for _, dt := range batch.Deposits { dataStrings = append(dataStrings, depositToStrings(dt)...) - if bytes.Equal(dt.Data, ethmultiversx.MissingCallData) { + if bytes.Equal(dt.Data, []byte{ethmultiversx.MissingDataProtocolMarker}) { continue } extraGas += (uint64(len(dt.Data))*2 + 1) * args.GasMapConfig.ScCallPerByte diff --git a/clients/multiversx/mxClientDataGetter_test.go b/clients/multiversx/mxClientDataGetter_test.go index c20e6b03..a89231ab 100644 --- a/clients/multiversx/mxClientDataGetter_test.go +++ b/clients/multiversx/mxClientDataGetter_test.go @@ -603,14 +603,14 @@ func TestMXClientDataGetter_WasProposedTransfer(t *testing.T) { hex.EncodeToString([]byte("converted_token1")), hex.EncodeToString(big.NewInt(2).Bytes()), hex.EncodeToString(big.NewInt(1).Bytes()), - hex.EncodeToString(ethmultiversx.MissingCallData), + hex.EncodeToString([]byte{ethmultiversx.MissingDataProtocolMarker}), hex.EncodeToString([]byte("from2")), hex.EncodeToString([]byte("to2")), hex.EncodeToString([]byte("converted_token2")), hex.EncodeToString(big.NewInt(4).Bytes()), hex.EncodeToString(big.NewInt(3).Bytes()), - hex.EncodeToString(ethmultiversx.MissingCallData), + hex.EncodeToString([]byte{ethmultiversx.MissingDataProtocolMarker}), } assert.Equal(t, expectedArgs, vmRequest.Args) @@ -661,7 +661,7 @@ func TestMXClientDataGetter_WasProposedTransfer(t *testing.T) { hex.EncodeToString([]byte("converted_token2")), hex.EncodeToString(big.NewInt(4).Bytes()), hex.EncodeToString(big.NewInt(3).Bytes()), - hex.EncodeToString(ethmultiversx.MissingCallData), + hex.EncodeToString([]byte{ethmultiversx.MissingDataProtocolMarker}), } assert.Equal(t, expectedArgs, vmRequest.Args) @@ -778,14 +778,14 @@ func TestMXClientDataGetter_GetActionIDForProposeTransfer(t *testing.T) { hex.EncodeToString([]byte("converted_token1")), hex.EncodeToString(big.NewInt(2).Bytes()), hex.EncodeToString(big.NewInt(1).Bytes()), - hex.EncodeToString(ethmultiversx.MissingCallData), + hex.EncodeToString([]byte{ethmultiversx.MissingDataProtocolMarker}), hex.EncodeToString([]byte("from2")), hex.EncodeToString([]byte("to2")), hex.EncodeToString([]byte("converted_token2")), hex.EncodeToString(big.NewInt(4).Bytes()), hex.EncodeToString(big.NewInt(3).Bytes()), - hex.EncodeToString(ethmultiversx.MissingCallData), + hex.EncodeToString([]byte{ethmultiversx.MissingDataProtocolMarker}), } assert.Equal(t, expectedArgs, vmRequest.Args) @@ -836,7 +836,7 @@ func TestMXClientDataGetter_GetActionIDForProposeTransfer(t *testing.T) { hex.EncodeToString([]byte("converted_token2")), hex.EncodeToString(big.NewInt(4).Bytes()), hex.EncodeToString(big.NewInt(3).Bytes()), - hex.EncodeToString(ethmultiversx.MissingCallData), + hex.EncodeToString([]byte{ethmultiversx.MissingDataProtocolMarker}), } assert.Equal(t, expectedArgs, vmRequest.Args) diff --git a/integrationTests/relayers/ethToMultiversX_test.go b/integrationTests/relayers/ethToMultiversX_test.go index b0260048..b3204a43 100644 --- a/integrationTests/relayers/ethToMultiversX_test.go +++ b/integrationTests/relayers/ethToMultiversX_test.go @@ -167,14 +167,14 @@ func testRelayersShouldExecuteTransfersFromEthToMultiversX(t *testing.T, withNat assert.Equal(t, value1, transfer.Transfers[0].Amount) assert.Equal(t, depositor1, common.BytesToAddress(transfer.Transfers[0].From)) assert.Equal(t, txNonceOnEthereum+1, transfer.Transfers[0].Nonce.Uint64()) - assert.Equal(t, ethmultiversx.MissingCallData, transfer.Transfers[0].Data) + assert.Equal(t, []byte{ethmultiversx.MissingDataProtocolMarker}, transfer.Transfers[0].Data) assert.Equal(t, destination2.AddressBytes(), transfer.Transfers[1].To) assert.Equal(t, hex.EncodeToString([]byte(ticker2)), transfer.Transfers[1].Token) assert.Equal(t, value2, transfer.Transfers[1].Amount) assert.Equal(t, depositor2, common.BytesToAddress(transfer.Transfers[1].From)) assert.Equal(t, txNonceOnEthereum+2, transfer.Transfers[1].Nonce.Uint64()) - assert.Equal(t, ethmultiversx.MissingCallData, transfer.Transfers[1].Data) + assert.Equal(t, []byte{ethmultiversx.MissingDataProtocolMarker}, transfer.Transfers[1].Data) } func TestRelayersShouldExecuteTransferFromEthToMultiversXHavingTxsWithSCcalls(t *testing.T) { @@ -192,8 +192,8 @@ func TestRelayersShouldExecuteTransferFromEthToMultiversXHavingTxsWithSCcalls(t }) t.Run("no SC call", func(t *testing.T) { testArgs := argsForSCCallsTest{ - providedScCallData: string(ethmultiversx.MissingCallData), - expectedScCallData: string(ethmultiversx.MissingCallData), + providedScCallData: string([]byte{ethmultiversx.MissingDataProtocolMarker}), + expectedScCallData: string([]byte{ethmultiversx.MissingDataProtocolMarker}), } testRelayersShouldExecuteTransferFromEthToMultiversXHavingTxsWithSCcalls(t, testArgs) @@ -350,14 +350,14 @@ func testRelayersShouldExecuteTransferFromEthToMultiversXHavingTxsWithSCcalls(t assert.Equal(t, value1, transfer.Transfers[0].Amount) assert.Equal(t, depositor1, common.BytesToAddress(transfer.Transfers[0].From)) assert.Equal(t, txNonceOnEthereum+1, transfer.Transfers[0].Nonce.Uint64()) - assert.Equal(t, ethmultiversx.MissingCallData, transfer.Transfers[0].Data) + assert.Equal(t, []byte{ethmultiversx.MissingDataProtocolMarker}, transfer.Transfers[0].Data) assert.Equal(t, destination2.AddressBytes(), transfer.Transfers[1].To) assert.Equal(t, hex.EncodeToString([]byte(ticker2)), transfer.Transfers[1].Token) assert.Equal(t, value2, transfer.Transfers[1].Amount) assert.Equal(t, depositor2, common.BytesToAddress(transfer.Transfers[1].From)) assert.Equal(t, txNonceOnEthereum+2, transfer.Transfers[1].Nonce.Uint64()) - assert.Equal(t, ethmultiversx.MissingCallData, transfer.Transfers[1].Data) + assert.Equal(t, []byte{ethmultiversx.MissingDataProtocolMarker}, transfer.Transfers[1].Data) assert.Equal(t, destination3Sc.AddressBytes(), transfer.Transfers[2].To) assert.Equal(t, hex.EncodeToString([]byte(ticker3)), transfer.Transfers[2].Token)