Skip to content

Commit

Permalink
Merge pull request #286 from multiversx/MX-15180-minor-refactor
Browse files Browse the repository at this point in the history 
Minor refactor for the ConvertToDisplayableData function
  • Loading branch information
@iulianpascalau
iulianpascalau authored Feb 26, 2024
2 parents bb69b9d + 2920035 commit dcc84c8
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 92 deletions.
126 changes: 77 additions & 49 deletions bridges/ethMultiversX/bridgeExecutor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -496,7 +501,7 @@ func (executor *bridgeExecutor) addMetadataToTransfer(transfer *clients.DepositT
}
}

transfer.Data = MissingCallData
transfer.Data = []byte{MissingDataProtocolMarker}
transfer.DisplayableData = ""

return transfer
Expand Down Expand Up @@ -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
}
33 changes: 9 additions & 24 deletions bridges/ethMultiversX/bridgeExecutor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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())
}
2 changes: 1 addition & 1 deletion clients/multiversx/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
12 changes: 6 additions & 6 deletions clients/multiversx/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions clients/multiversx/mxClientDataGetter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions integrationTests/relayers/ethToMultiversX_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit dcc84c8

Please sign in to comment.