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

Fix backwards compatibility #285

Merged
merged 6 commits into from
Feb 26, 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
141 changes: 121 additions & 20 deletions bridges/ethMultiversX/bridgeExecutor.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package ethmultiversx

import (
"bytes"
"context"
"encoding/hex"
"encoding/binary"
"fmt"
"math/big"
"strings"
"time"

"github.com/ethereum/go-ethereum/common"
Expand All @@ -20,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 for wrongly initiated contract calls that do not contain data
const MissingCallData = "<missing call data>"
// 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 @@ -468,10 +475,6 @@ func (executor *bridgeExecutor) addBatchSCMetadata(ctx context.Context, transfer
return nil, ErrNilBatch
}

if !executor.hasSCCalls(transfers) {
return transfers, nil
}

events, err := executor.ethereumClient.GetBatchSCMetadata(ctx, transfers.ID)
if err != nil {
return nil, err
Expand All @@ -488,26 +491,20 @@ func (executor *bridgeExecutor) addMetadataToTransfer(transfer *clients.DepositT
for _, event := range events {
if event.DepositNonce == transfer.Nonce {
transfer.Data = []byte(event.CallData)
if len(transfer.Data) == 0 {
// will add a dummy data so the relayers won't panic
transfer.Data = []byte(MissingCallData)
var err error
transfer.DisplayableData, err = ConvertToDisplayableData(transfer.Data)
if err != nil {
executor.log.Warn("failed to convert call data to displayable data", "error", err)
}
transfer.DisplayableData = hex.EncodeToString(transfer.Data)

return transfer
}
}
return transfer
}

func (executor *bridgeExecutor) hasSCCalls(transfers *clients.TransferBatch) bool {
for _, t := range transfers.Deposits {
if executor.ethereumClient.IsDepositSCCall(t) {
return true
}
}
transfer.Data = []byte{MissingDataProtocolMarker}
transfer.DisplayableData = ""

return false
return transfer
}

// WasTransferPerformedOnEthereum returns true if the batch was performed on Ethereum
Expand Down Expand Up @@ -717,3 +714,107 @@ func (executor *bridgeExecutor) CheckEthereumClientAvailability(ctx context.Cont
func (executor *bridgeExecutor) IsInterfaceNil() bool {
return executor == nil
}

// ConvertToDisplayableData
/** @param callData The encoded data specifying the cross-chain call details. The expected format is:
* 0x01 + endpoint_name_length (4 bytes) + endpoint_name + gas_limit (8 bytes) +
* num_arguments_length (4 bytes) + [argument_length (4 bytes) + argument]...
* This payload includes the endpoint name, gas limit for the execution, and the arguments for the call.
*/
func ConvertToDisplayableData(callData []byte) (string, error) {
dragos-rebegea marked this conversation as resolved.
Show resolved Hide resolved
if len(callData) == 0 {
return "", fmt.Errorf("callData too short for protocol indicator")
}

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)
}
}

func convertBytesToDisplayableData(callData []byte) (string, error) {
callData, endpointName, err := extractString(callData)
if err != nil {
return "", fmt.Errorf("%w for endpoint", err)
}

callData, gasLimit, err := extractGasLimit(callData)
if err != nil {
return "", err
}

callData, numArgumentsLength, err := extractArgumentsLen(callData)
if err != nil {
return "", err
}

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)
}

return fmt.Sprintf("Endpoint: %s, Gas: %d, Arguments: %s", endpointName, gasLimit, strings.Join(arguments, "@")), nil
}

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

// 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
}

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")
}

gasLimitLength := int(binary.BigEndian.Uint64(callData[:uint64ArgBytes]))
callData = callData[uint64ArgBytes:]

// Check for gas limit
if len(callData) < gasLimitLength {
return nil, 0, fmt.Errorf("callData too short for gas limit")
}

gasLimitBytes := append(bytes.Repeat([]byte{0x00}, 8-int(gasLimitLength)), callData[:gasLimitLength]...)
gasLimit := binary.BigEndian.Uint64(gasLimitBytes)
callData = callData[gasLimitLength:] // remove the gas limit bytes

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 callData, length, nil
}
162 changes: 154 additions & 8 deletions bridges/ethMultiversX/bridgeExecutor_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ethmultiversx

import (
"bytes"
"context"
"errors"
"fmt"
Expand All @@ -19,6 +20,7 @@ import (
"github.com/multiversx/mx-chain-core-go/core/check"
logger "github.com/multiversx/mx-chain-logger-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var expectedErr = errors.New("expected error")
Expand Down Expand Up @@ -388,9 +390,6 @@ func TestEthToMultiversXBridgeExecutor_GetAndStoreBatchFromEthereum(t *testing.T
CallData: depositData,
}}, nil
},
IsDepositSCCallCalled: func(deposit *clients.DepositTransfer) bool {
return deposit.Nonce == depositNonce
},
}
executor, _ := NewBridgeExecutor(args)
err := executor.GetAndStoreBatchFromEthereum(context.Background(), providedNonce)
Expand All @@ -399,7 +398,7 @@ func TestEthToMultiversXBridgeExecutor_GetAndStoreBatchFromEthereum(t *testing.T
assert.True(t, expectedBatch == executor.GetStoredBatch()) // pointer testing
assert.Equal(t, depositData, string(executor.batch.Deposits[0].Data))
})
t.Run("should add deposits metadata for sc calls even if with invalid data", func(t *testing.T) {
t.Run("should add deposits metadata for sc calls even if with no data", func(t *testing.T) {
args := createMockExecutorArgs()
providedNonce := uint64(8346)
depositNonce := uint64(100)
Expand All @@ -423,16 +422,13 @@ func TestEthToMultiversXBridgeExecutor_GetAndStoreBatchFromEthereum(t *testing.T
CallData: depositData,
}}, nil
},
IsDepositSCCallCalled: func(deposit *clients.DepositTransfer) bool {
return deposit.Nonce == depositNonce
},
}
executor, _ := NewBridgeExecutor(args)
err := executor.GetAndStoreBatchFromEthereum(context.Background(), providedNonce)

assert.Nil(t, err)
assert.True(t, expectedBatch == executor.GetStoredBatch()) // pointer testing
assert.Equal(t, MissingCallData, string(executor.batch.Deposits[0].Data))
assert.Equal(t, "", string(executor.batch.Deposits[0].Data))
})
}

Expand Down Expand Up @@ -1865,3 +1861,153 @@ func TestBridgeExecutor_CheckAvailableTokens(t *testing.T) {
assert.True(t, accumulatedBurnedTokensCalled)
})
}

func TestConvertToDisplayableData_EmptyCallData(t *testing.T) {
t.Parallel()

callData := []byte{0x00}
want := ""
got, err := ConvertToDisplayableData(callData)
require.Nil(t, err)
require.Equal(t, want, got)
}

func TestConvertToDisplayableData_ValidCallDataWithNoArguments(t *testing.T) {
t.Parallel()

callData := func() []byte {
b := []byte{0x01, 0x00, 0x00, 0x00, 0x03, 'a', 'b', 'c'}
b = append(b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0, 0x04) // Gas limit length
b = append(b, 0x1D, 0xCD, 0x65, 0x00) // Gas limit
b = append(b, 0x00, 0x00, 0x00, 0x00) // numArguments
return b
}()

want := "Endpoint: abc, Gas: 500000000, Arguments: "
got, err := ConvertToDisplayableData(callData)
require.Nil(t, err)
require.Equal(t, want, got)
}

func TestConvertToDisplayableData_MultipleTypesArguments(t *testing.T) {
t.Parallel()

callData := func() []byte {
b := []byte{0x01, 0x00, 0x00, 0x00, 0x03, 'a', 'b', 'c'}
b = append(b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0, 0x04) // Gas limit length
b = append(b, 0x1D, 0xCD, 0x65, 0x00) // Gas limit
b = append(b, 0x00, 0x00, 0x00, 0x02) // numArguments
b = append(b, 0x00, 0x00, 0x00, 0x05) // Argument 0 length
b = append(b, bytes.Repeat([]byte{'A'}, 5)...) // Argument 0 data
b = append(b, 0x00, 0x00, 0x00, 0x32) // Argument 1 length
b = append(b, bytes.Repeat([]byte{'B'}, 50)...) // Argument 1 data
return b
}()

want := "Endpoint: abc, Gas: 500000000, Arguments: AAAAA@BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"
got, err := ConvertToDisplayableData(callData)
require.Nil(t, err)
if got != want {
t.Errorf("Test with multiple types arguments failed. Got: %v, want: %v", got, want)
}
}

func TestConvertToDisplayableData_TooShortForProtocolIndicator(t *testing.T) {
t.Parallel()
_, err := ConvertToDisplayableData([]byte{})
require.NotNil(t, err)
if err != nil {
require.Equal(t, "callData too short for protocol indicator", err.Error())
}
}

func TestConvertToDisplayableData_TooShortForEndpointNameLength(t *testing.T) {
t.Parallel()
_, err := ConvertToDisplayableData([]byte{0x01})
require.NotNil(t, err)
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)
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)
require.Equal(t, "callData too short while extracting the string data for endpoint", err.Error())
}

func TestConvertToDisplayableData_TooShortForGasLimitLength(t *testing.T) {
t.Parallel()
callData := func() []byte {
b := []byte{0x01, 0x00, 0x00, 0x00, 0x03, 'a', 'b', 'c'}
b = append(b, 0x00, 0x00, 0x04) // Bad Gas limit length
return b
}()
_, err := ConvertToDisplayableData(callData)
require.NotNil(t, err)
require.Equal(t, "callData too short for gas limit length", err.Error())
}

func TestConvertToDisplayableData_TooShortForGasLimit(t *testing.T) {
t.Parallel()
callData := func() []byte {
b := []byte{0x01, 0x00, 0x00, 0x00, 0x03, 'a', 'b', 'c'}
b = append(b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0, 0x04) // Gas limit length
b = append(b, 0x03, 0x00, 0x03) // Bad Gas limit
return b
}()
_, err := ConvertToDisplayableData(callData)
require.NotNil(t, err)
require.Equal(t, "callData too short for gas limit", err.Error())
}

func TestConvertToDisplayableData_TooShortForNumberOfArgumentsLength(t *testing.T) {
t.Parallel()
callData := func() []byte {
b := []byte{0x01, 0x00, 0x00, 0x00, 0x03, 'a', 'b', 'c'}
b = append(b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0, 0x04) // Gas limit length
b = append(b, 0x1D, 0xCD, 0x65, 0x00) // Gas limit
b = append(b, 0x00, 0x00, 0x03) // Bad numArgument
return b
}()
_, err := ConvertToDisplayableData(callData)
require.NotNil(t, err)
require.Equal(t, "callData too short for numArguments length", err.Error())
}

func TestConvertToDisplayableData_TooShortForArgumentLength(t *testing.T) {
t.Parallel()
callData := func() []byte {
b := []byte{0x01, 0x00, 0x00, 0x00, 0x03, 'a', 'b', 'c'}
b = append(b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0, 0x04) // Gas limit length
b = append(b, 0x1D, 0xCD, 0x65, 0x00) // Gas limit
b = append(b, 0x00, 0x00, 0x00, 0x01) // numArguments
b = append(b, 0x00, 0x00, 0x04) // Bad Argument 0 length
return b
}()
_, err := ConvertToDisplayableData(callData)
require.NotNil(t, err)
require.Equal(t, "callData too short while extracting the length for argument 0", err.Error())
}

func TestConvertToDisplayableData_TooShortForArgumentData(t *testing.T) {
t.Parallel()
callData := func() []byte {
b := []byte{0x01, 0x00, 0x00, 0x00, 0x03, 'a', 'b', 'c'}
b = append(b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0, 0x04) // Gas limit length
b = append(b, 0x1D, 0xCD, 0x65, 0x00) // Gas limit
b = append(b, 0x00, 0x00, 0x00, 0x01) // numArguments
b = append(b, 0x00, 0x00, 0x00, 0x04) // Argument 0 length
b = append(b, 0x00, 0x00, 0x04) // Bad Argument 0 data
return b
}()
_, err := ConvertToDisplayableData(callData)
require.NotNil(t, err)
require.Equal(t, "callData too short while extracting the string data for argument 0", err.Error())
}
1 change: 0 additions & 1 deletion bridges/ethMultiversX/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ type MultiversXClient interface {
type EthereumClient interface {
GetBatch(ctx context.Context, nonce uint64) (*clients.TransferBatch, error)
WasExecuted(ctx context.Context, batchID uint64) (bool, error)
IsDepositSCCall(deposit *clients.DepositTransfer) bool
GenerateMessageHash(batch *batchProcessor.ArgListsBatch, batchId uint64) (common.Hash, error)

BroadcastSignatureForMessageHash(msgHash common.Hash)
Expand Down
Loading
Loading