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.

Sign up for GitHub

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

Mint burn tokens #266

Merged
merged 11 commits into from
Dec 22, 2023
Merged

Mint burn tokens #266

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6b6a315
do the balance check on for non mintBurn tokens
dragos-rebegea Nov 15, 2023
48c98ac
Merge remote-tracking branch 'origin/main' into main
dragos-rebegea Nov 15, 2023
b190a3c
add mintBurn check for multiversx2ethereum
dragos-rebegea Nov 28, 2023
3f245a3
update clients stub
dragos-rebegea Dec 4, 2023
db6acdc
update dependencies and update stubs
dragos-rebegea Dec 6, 2023
307e0db
fixes after review
dragos-rebegea Dec 7, 2023
d16bb1c
fix linter
dragos-rebegea Dec 7, 2023
dce9bdd
Merge remote-tracking branch 'origin/feat/native-esdts-bridging' into…
dragos-rebegea Dec 20, 2023
6b556f4
fixes after review
dragos-rebegea Dec 20, 2023
d6580c8
fixes after review
dragos-rebegea Dec 20, 2023
a8c7e8c
fixes after review
dragos-rebegea Dec 21, 2023
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
121 changes: 118 additions & 3 deletions bridges/ethMultiversX/bridgeExecutor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package ethmultiversx
import (
"context"
"fmt"
"github.com/multiversx/mx-bridge-eth-go/core/batchProcessor"
Copy link
Contributor

Choose a reason for hiding this comment

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

go imports

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"math/big"
"time"

"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -178,7 +180,7 @@ func (executor *bridgeExecutor) GetLastExecutedEthBatchIDFromMultiversX(ctx cont
return batchID, err
}

// VerifyLastDepositNonceExecutedOnEthereumBatch will check the deposit nonces from the fetched batch from Ethereum client
// VerifyLastDepositNonceExecutedOnEthereumBatch will check the deposit Nonces from the fetched batch from Ethereum client
func (executor *bridgeExecutor) VerifyLastDepositNonceExecutedOnEthereumBatch(ctx context.Context) error {
if executor.batch == nil {
return ErrNilBatch
Expand Down Expand Up @@ -403,6 +405,7 @@ func (executor *bridgeExecutor) PerformActionOnMultiversX(ctx context.Context) e
return ErrNilBatch
}

// TODO: check mintBurn balances before performing the action
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO will be implemented in next PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

hash, err := executor.multiversXClient.PerformAction(ctx, executor.actionID, executor.batch)
if err != nil {
return err
Expand Down Expand Up @@ -467,7 +470,12 @@ func (executor *bridgeExecutor) SignTransferOnEthereum() error {
return ErrNilBatch
}

hash, err := executor.ethereumClient.GenerateMessageHash(executor.batch)
argLists, err := batchProcessor.ExtractList(executor.batch)
if err != nil {
return err
}

hash, err := executor.ethereumClient.GenerateMessageHash(argLists, executor.batch.ID)
if err != nil {
return err
}
Expand All @@ -493,7 +501,19 @@ func (executor *bridgeExecutor) PerformTransferOnEthereum(ctx context.Context) e

executor.log.Debug("fetched quorum size", "quorum", quorumSize.Int64())

hash, err := executor.ethereumClient.ExecuteTransfer(ctx, executor.msgHash, executor.batch, int(quorumSize.Int64()))
argLists, err := batchProcessor.ExtractList(executor.batch)
if err != nil {
return err
}

err = executor.checkAvailableTokensOnEthereum(ctx, argLists.Tokens, argLists.ConvertedTokenBytes, argLists.Amounts)
if err != nil {
return err
}

executor.log.Info("executing transfer " + executor.batch.String())

hash, err := executor.ethereumClient.ExecuteTransfer(ctx, executor.msgHash, argLists, executor.batch.ID, int(quorumSize.Int64()))
if err != nil {
return err
}
Expand All @@ -504,6 +524,101 @@ func (executor *bridgeExecutor) PerformTransferOnEthereum(ctx context.Context) e
return nil
}

func (executor *bridgeExecutor) checkCumulatedTransfers(ctx context.Context, tokens []common.Address, convertedTokens [][]byte, amounts []*big.Int) error {
for i, token := range tokens {
err := executor.checkToken(ctx, token, convertedTokens[i], amounts[i])
if err != nil {
return err
}
}
return nil
}

func (executor *bridgeExecutor) checkToken(ctx context.Context, token common.Address, convertedToken []byte, amount *big.Int) error {
isMintBurnToken, err := executor.isMintBurnToken(ctx, token, convertedToken)
if err != nil {
return err
}

if isMintBurnToken {
return executor.checkRequiredMintBurnBalance(ctx, token, convertedToken)
}

return executor.ethereumClient.CheckRequiredBalance(ctx, token, amount)
}

func (executor *bridgeExecutor) isMintBurnToken(ctx context.Context, token common.Address, convertedToken []byte) (bool, error) {
isMintBurnOnEthereum := executor.isMintBurnOnEthereum(ctx, token)
isMintBurnOnMultiversX := executor.isMintBurnOnMultiversX(ctx, convertedToken)
if isMintBurnOnEthereum != isMintBurnOnMultiversX {
return false, fmt.Errorf("%w isMintBurnOnEthereum = %v, isMintBurnOnMultiversX = %v", ErrInvalidSetupMintBurnToken, isMintBurnOnEthereum, isMintBurnOnMultiversX)
}
return isMintBurnOnEthereum, nil
}

func (executor *bridgeExecutor) checkRequiredMintBurnBalance(ctx context.Context, token common.Address, convertedToken []byte) error {
mintedBalance, err := executor.ethereumClient.TokenMintedBalances(ctx, token)
if err != nil {
return err
}

burntBalance, err := executor.multiversXClient.AccumulatedBurnedTokens(ctx, convertedToken)
if err != nil {
return err
}
if mintedBalance.Cmp(burntBalance) != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this ok? When will the burn be completed? At the moment of transfer in the mvx safe contract or when the transfer is completed on eth side and marked as finished on mvx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when the transfer is completed on eth side and marked as finished on mvx, thats why it should be equal at the moment of the batch execution

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, should work then

return fmt.Errorf("%w, minted: %s, burnt: %s for ERC20 token %s/ ESDT token %s",
ErrMintBurnBalance, mintedBalance.String(), burntBalance.String(), token.String(), convertedToken)
}
return nil
}

func (executor *bridgeExecutor) isMintBurnOnEthereum(ctx context.Context, erc20Address common.Address) bool {
isMintBurn, err := executor.ethereumClient.WhitelistedTokensMintBurn(ctx, erc20Address)
if err != nil {
return false
}
return isMintBurn
}

func (executor *bridgeExecutor) isMintBurnOnMultiversX(ctx context.Context, token []byte) bool {

isMintBurn, err := executor.multiversXClient.IsMintBurnAllowed(ctx, token)
if err != nil {
return false
}
return isMintBurn
}

func (executor *bridgeExecutor) checkAvailableTokensOnEthereum(ctx context.Context, tokens []common.Address, convertedTokens [][]byte, amounts []*big.Int) error {
tokens, convertedTokens, amounts = executor.getCumulatedTransfers(tokens, convertedTokens, amounts)

return executor.checkCumulatedTransfers(ctx, tokens, convertedTokens, amounts)
}

func (executor *bridgeExecutor) getCumulatedTransfers(tokens []common.Address, convertedTokens [][]byte, amounts []*big.Int) ([]common.Address, [][]byte, []*big.Int) {
cumulatedAmounts := make(map[common.Address]*big.Int)
uniqueTokens := make([]common.Address, 0)
uniqueConvertedTokens := make([][]byte, 0)

for i, token := range tokens {
if _, exists := cumulatedAmounts[token]; exists {
Copy link
Contributor

Choose a reason for hiding this comment

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

The code reads worse in this changeset. How about this:

for i, token := range tokens {
    existingValue, exists := cumulatedAmounts[token]
    if exists {
        existingValue.Add(existingValue, amounts[i])
        continue
    }

    cumulatedAmounts[token] = amounts[i]
    uniqueTokens = append(uniqueTokens, token)
    uniqueConvertedTokens = append(uniqueConvertedTokens, convertedTokens[i])

}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👍

cumulatedAmounts[token].Add(cumulatedAmounts[token], amounts[i])
} else {
cumulatedAmounts[token] = amounts[i]
uniqueTokens = append(uniqueTokens, token)
uniqueConvertedTokens = append(uniqueConvertedTokens, convertedTokens[i])
}
}

finalAmounts := make([]*big.Int, len(uniqueTokens))
for i, token := range uniqueTokens {
finalAmounts[i] = cumulatedAmounts[token]
}

return uniqueTokens, uniqueConvertedTokens, finalAmounts
}

// ProcessQuorumReachedOnEthereum returns true if the proposed transfer reached the set quorum
func (executor *bridgeExecutor) ProcessQuorumReachedOnEthereum(ctx context.Context) (bool, error) {
return executor.ethereumClient.IsQuorumReached(ctx, executor.msgHash)
Expand Down
34 changes: 29 additions & 5 deletions bridges/ethMultiversX/bridgeExecutor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"github.com/multiversx/mx-bridge-eth-go/core/batchProcessor"
Copy link
Contributor

Choose a reason for hiding this comment

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

go imports

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"math/big"
"strings"
"testing"
Expand Down Expand Up @@ -1083,7 +1084,7 @@ func TestMultiversXToEthBridgeExecutor_SignTransferOnEthereum(t *testing.T) {

args := createMockExecutorArgs()
args.EthereumClient = &bridgeTests.EthereumClientStub{
GenerateMessageHashCalled: func(batch *clients.TransferBatch) (common.Hash, error) {
GenerateMessageHashCalled: func(batch *batchProcessor.ArgListsBatch, batchID uint64) (common.Hash, error) {
return common.Hash{}, expectedErr
},
}
Expand All @@ -1100,7 +1101,7 @@ func TestMultiversXToEthBridgeExecutor_SignTransferOnEthereum(t *testing.T) {
wasCalledBroadcastSignatureForMessageHashCalled := false
args := createMockExecutorArgs()
args.EthereumClient = &bridgeTests.EthereumClientStub{
GenerateMessageHashCalled: func(batch *clients.TransferBatch) (common.Hash, error) {
GenerateMessageHashCalled: func(batch *batchProcessor.ArgListsBatch, batchID uint64) (common.Hash, error) {
wasCalledGenerateMessageHashCalled = true
return common.Hash{}, nil
},
Expand Down Expand Up @@ -1153,7 +1154,7 @@ func TestMultiversXToEthBridgeExecutor_PerformTransferOnEthereum(t *testing.T) {
GetQuorumSizeCalled: func(ctx context.Context) (*big.Int, error) {
return big.NewInt(0), nil
},
ExecuteTransferCalled: func(ctx context.Context, msgHash common.Hash, batch *clients.TransferBatch, quorum int) (string, error) {
ExecuteTransferCalled: func(ctx context.Context, msgHash common.Hash, batch *batchProcessor.ArgListsBatch, batchId uint64, quorum int) (string, error) {
return "", expectedErr
},
}
Expand All @@ -1176,9 +1177,16 @@ func TestMultiversXToEthBridgeExecutor_PerformTransferOnEthereum(t *testing.T) {
wasCalledGetQuorumSizeCalled = true
return big.NewInt(int64(providedQuorum)), nil
},
ExecuteTransferCalled: func(ctx context.Context, msgHash common.Hash, batch *clients.TransferBatch, quorum int) (string, error) {
ExecuteTransferCalled: func(ctx context.Context, msgHash common.Hash, batch *batchProcessor.ArgListsBatch, batchId uint64, quorum int) (string, error) {
assert.True(t, providedHash == msgHash)
assert.True(t, providedBatch == batch)
assert.True(t, providedBatch.ID == batchId)
for i := 0; i < len(providedBatch.Deposits); i++ {
assert.True(t, providedBatch.Deposits[i].Amount == batch.Amounts[i])
Copy link
Contributor

Choose a reason for hiding this comment

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

can use assert.Equal instead of assert.True as they will be evaluated by value not pointer. Also, the test will show a clearer image of what is wrong with assert.Equal. Valid here and on all asserts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assert.Equal used instead, also no need for extra convertion to string. good suggestion

assert.True(t, providedBatch.Deposits[i].Nonce == batch.Nonces[i].Uint64())
assert.True(t, CompareBytes(providedBatch.Deposits[i].ToBytes, batch.Recipients[i].Bytes()))
assert.True(t, CompareBytes(providedBatch.Deposits[i].TokenBytes, batch.Tokens[i].Bytes()))
assert.True(t, CompareBytes(providedBatch.Deposits[i].ConvertedTokenBytes, batch.ConvertedTokenBytes[i]))
}
assert.True(t, providedQuorum == quorum)

wasCalledExecuteTransferCalled = true
Expand All @@ -1196,6 +1204,9 @@ func TestMultiversXToEthBridgeExecutor_PerformTransferOnEthereum(t *testing.T) {
})
}

func TestMultiversXToEthBridgeExecutor_checkAvailableTokens(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

empty test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed


}
func TestMultiversXToEthBridgeExecutor_IsQuorumReachedOnEthereum(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1655,3 +1666,16 @@ func TestBridgeExecutor_ValidateBatch(t *testing.T) {
assert.True(t, result)
assert.True(t, validateBatchCalled)
}

// CompareBytes compares two byte slices and returns true if they are equal.
func CompareBytes(a, b []byte) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

bytes.Equal(b1, b2) instead of this? or simply string(b1) == string(b2) (which bytes.Equal also does)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

if len(a) != len(b) {
return false
}
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}
6 changes: 6 additions & 0 deletions bridges/ethMultiversX/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,9 @@ var ErrNilSignaturesHolder = errors.New("nil signatures holder")

// ErrNilBatchValidator signals that a nil batch validator was provided
var ErrNilBatchValidator = errors.New("nil batch validator")

// ErrInvalidSetupMintBurnToken signals that an invalid setup mint burn token was provided
var ErrInvalidSetupMintBurnToken = errors.New("invalid setup mint burn token")

// ErrMintBurnBalance signals that the mint burn balances are not expected
var ErrMintBurnBalance = errors.New("mint burn balances are not expected")
10 changes: 8 additions & 2 deletions bridges/ethMultiversX/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ethmultiversx

import (
"context"
"github.com/multiversx/mx-bridge-eth-go/core/batchProcessor"
Copy link
Contributor

Choose a reason for hiding this comment

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

go imports

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"math/big"

"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -29,6 +30,8 @@ type MultiversXClient interface {
WasSigned(ctx context.Context, actionID uint64) (bool, error)
PerformAction(ctx context.Context, actionID uint64, batch *clients.TransferBatch) (string, error)
CheckClientAvailability(ctx context.Context) error
IsMintBurnAllowed(ctx context.Context, token []byte) (bool, error)
AccumulatedBurnedTokens(ctx context.Context, token []byte) (*big.Int, error)
Close() error
IsInterfaceNil() bool
}
Expand All @@ -37,14 +40,17 @@ type MultiversXClient interface {
type EthereumClient interface {
GetBatch(ctx context.Context, nonce uint64) (*clients.TransferBatch, error)
WasExecuted(ctx context.Context, batchID uint64) (bool, error)
GenerateMessageHash(batch *clients.TransferBatch) (common.Hash, error)
GenerateMessageHash(batch *batchProcessor.ArgListsBatch, batchId uint64) (common.Hash, error)

BroadcastSignatureForMessageHash(msgHash common.Hash)
ExecuteTransfer(ctx context.Context, msgHash common.Hash, batch *clients.TransferBatch, quorum int) (string, error)
ExecuteTransfer(ctx context.Context, msgHash common.Hash, batch *batchProcessor.ArgListsBatch, batchId uint64, quorum int) (string, error)
GetTransactionsStatuses(ctx context.Context, batchId uint64) ([]byte, error)
GetQuorumSize(ctx context.Context) (*big.Int, error)
IsQuorumReached(ctx context.Context, msgHash common.Hash) (bool, error)
CheckClientAvailability(ctx context.Context) error
CheckRequiredBalance(ctx context.Context, erc20Address common.Address, value *big.Int) error
TokenMintedBalances(ctx context.Context, token common.Address) (*big.Int, error)
WhitelistedTokensMintBurn(ctx context.Context, token common.Address) (bool, error)
IsInterfaceNil() bool
}

Expand Down
Loading