Add more checks #375
Add more checks #375
Conversation
| if params.IsNativeOnEth { | ||
| burnAmount := initialSupply | ||
| tx2, err2 = handler.SafeContract.InitSupplyMintBurn(auth, erc20Address, zeroValueBigInt, burnAmount) | ||
| } else { | ||
| mintAmount := initialSupply | ||
| tx2, err2 = handler.SafeContract.InitSupplyMintBurn(auth, erc20Address, mintAmount, zeroValueBigInt) | ||
| } |
There was a problem hiding this comment.
perhaps extract txInitSupplyMintBurn, errInitSupplyMintBurn := handler.SafeContract.InitSupplyMintBurn(auth, erc20Address, mintAmount, burnAmount) to be called only once after this if and only set minAmount and burnAmount here
| @@ -492,6 +500,27 @@ func (handler *EthereumHandler) Mint(ctx context.Context, params TestTokenParams | |||
| handler.checkEthTxResult(ctx, tx.Hash()) | |||
| } | |||
|
|
|||
| func (handler *EthereumHandler) GetTotalBalancesForToken(ctx context.Context, address common.Address) *big.Int { | |||
There was a problem hiding this comment.
missing comment on all new methods
| if params.IsNativeOnMvX { | ||
| burnAmount := initialSupply | ||
| hash, txResult := handler.ChainSimulator.ScCall( | ||
| ctx, | ||
| handler.OwnerKeys.MvxSk, | ||
| handler.MultisigAddress, | ||
| zeroStringValue, | ||
| setCallsGasLimit, | ||
| initSupplyMintBurnEsdtSafe, | ||
| []string{ | ||
| hex.EncodeToString([]byte(tkData.MvxChainSpecificToken)), | ||
| hex.EncodeToString(zeroValueBigInt.Bytes()), | ||
| hex.EncodeToString(burnAmount.Bytes()), | ||
| }, | ||
| ) | ||
| log.Info("initial supply tx executed", "hash", hash, "status", txResult.Status, | ||
| "initial mint", "0", "initial burned", params.InitialSupplyValue) | ||
| } else { | ||
| mintAmount := initialSupply | ||
| hash, txResult := handler.ChainSimulator.ScCall( | ||
| ctx, | ||
| handler.OwnerKeys.MvxSk, | ||
| handler.MultisigAddress, | ||
| zeroStringValue, | ||
| setCallsGasLimit, | ||
| initSupplyMintBurnEsdtSafe, | ||
| []string{ | ||
| hex.EncodeToString([]byte(tkData.MvxChainSpecificToken)), | ||
| hex.EncodeToString(mintAmount.Bytes()), | ||
| hex.EncodeToString(zeroValueBigInt.Bytes()), | ||
| }, | ||
| ) | ||
| log.Info("initial supply tx executed", "hash", hash, "status", txResult.Status, | ||
| "initial mint", params.InitialSupplyValue, "initial burned", "0") | ||
| } |
There was a problem hiding this comment.
same here, you can call ScCall only once, just set the proper variables inside ifs
| @@ -1043,6 +1066,33 @@ func (handler *MultiversxHandler) TransferToken(ctx context.Context, source Keys | |||
| "hash", hash, "status", txResult.Status) | |||
| } | |||
|
|
|||
| func (handler *MultiversxHandler) GetTotalBalancesForToken(ctx context.Context, token string) *big.Int { | |||
There was a problem hiding this comment.
missing comments on new methods
| hex.EncodeToString([]byte(token)), | ||
| } | ||
| responseData := handler.ChainSimulator.ExecuteVMQuery(ctx, handler.SafeAddress, getTotalBalances, queryParams) | ||
| value := big.NewInt(0).SetBytes(responseData[0]) |
There was a problem hiding this comment.
is there any chance that responseData is nil here, resulting in a panic while trying to access index 0?
| @@ -492,6 +500,27 @@ func (handler *EthereumHandler) Mint(ctx context.Context, params TestTokenParams | |||
| handler.checkEthTxResult(ctx, tx.Hash()) | |||
| } | |||
|
|
|||
| func (handler *EthereumHandler) GetTotalBalancesForToken(ctx context.Context, address common.Address) *big.Int { | |||
There was a problem hiding this comment.
missing comments on exported items. Here, on L510 & L517
integrationTests/relayers/slowTests/framework/ethereumHandler.go
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| hash, txResult := handler.ChainSimulator.ScCall( |
| } | ||
|
|
||
| return setup.checkMvxLockedBalanceForToken(params, secondBridge) |
There was a problem hiding this comment.
mismatch between the function name and this provided parameter. Also on the next function
cosmatudor
dismissed
sstanculeanu’s stale review
The base branch was changed.
| if len(operation.MvxSCCallData) > 0 || operation.MvxForceSCCall { | ||
| if operation.MvxFaultySCCall { | ||
| // the balance should be bridged back to the receiver on Ethereum - fee | ||
| totalRefund.Add(totalRefund, operation.ValueToTransferToMvx) | ||
| totalRefund.Sub(totalRefund, feeInt) | ||
| } | ||
| } |
There was a problem hiding this comment.
| if len(operation.MvxSCCallData) > 0 || operation.MvxForceSCCall { | |
| if operation.MvxFaultySCCall { | |
| // the balance should be bridged back to the receiver on Ethereum - fee | |
| totalRefund.Add(totalRefund, operation.ValueToTransferToMvx) | |
| totalRefund.Sub(totalRefund, feeInt) | |
| } | |
| } | |
| if !operation.MvxForceSCCall{ | |
| continue | |
| } | |
| if len(operation.MvxSCCallData) == 0l { | |
| continue | |
| } | |
| if !operation.MvxFaultySCCall { | |
| continue | |
| } | |
| // the balance should be bridged back to the receiver on Ethereum - fee | |
| totalRefund.Add(totalRefund, operation.ValueToTransferToMvx) | |
| totalRefund.Sub(totalRefund, feeInt) |
There was a problem hiding this comment.
I'm not sure these 2 versions behave the same. The condition from this if:
if len(operation.MvxSCCallData) > 0 || operation.MvxForceSCCall {
is passed if one of those two assertions is true.
On the other hand, using ifs in the suggested order behaves as if both conditions (operation.MvxForceSCCall being true and len(operation.MvxSCCallData) > 0) must be satisfied simultaneously
Working to refactor this logic
No description provided.