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

integration tests for esdt-safe contract #6543

Open
wants to merge 4 commits into
base: MX-15929-preps-esdt-safe
Choose a base branch
from

Conversation

axenteoctavian
Copy link

@axenteoctavian axenteoctavian commented Oct 17, 2024

Reasoning behind the pull request

  • esdt-safe contract integration tests with chain simulator
  • test sov -> main -> sov
  • test main -> sov -> main

Proposed changes

Testing procedure

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@axenteoctavian axenteoctavian self-assigned this Oct 17, 2024
@axenteoctavian axenteoctavian marked this pull request as ready for review October 17, 2024 12:08
@mariusmihaic mariusmihaic self-requested a review October 18, 2024 08:47
@@ -114,6 +115,18 @@ func deployBridgeSetup(
}
}

func esdtSafeContract(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why even make this func just to add an extra @? Simply use DeployContract

// 2. transfer from main chain to sovereign chain
// 3. transfer again the same tokens from sovereign chain to main chain
// tokens are originated from sovereign chain
// esdt-safe contract in main chain will issue its own tokens ar registerToken step
Copy link
Contributor

Choose a reason for hiding this comment

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

ar typo


receiverShardId := uint32(0)

// TODO uncomment dynamic tokens, currently there is no issue function in SC framework for dynamic esdts
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a Jira task for this.

}
txResult = deposit(t, cs, receiver.Bytes, &receiverNonce, bridgeData.ESDTSafeAddress, []chainSim.ArgsDepositToken{depositToken}, wallet.Bytes)
chainSim.RequireSuccessfulTransaction(t, txResult)
chainSim.RequireAccountHasToken(t, cs, getTokenIdentifier(receivedToken), receiver.Bech32, big.NewInt(0).Sub(receivedToken.Amount, big.NewInt(1)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we simply check the account has 0 balance?

tokenSupply, err := cs.GetNodeHandler(esdtSafeAddrShard).GetFacadeHandler().GetTokenSupply(getTokenIdentifier(depositToken))
require.Nil(t, err)
require.NotNil(t, tokenSupply)
require.Equal(t, depositAmount.String(), tokenSupply.Burned)
Copy link
Contributor

Choose a reason for hiding this comment

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

[question]
How come this token's supply is always burned, yet the contract keeps one extra for for (dynamic) SFT/MetaESDT?
Is that kept one a different one that the contract mints?

return strings.Split(tokenIdentifier, "-")[1]
}

func generateHelloInAllShards(t *testing.T, cs chainSim.ChainSimulator) map[uint32]dtos.WalletAddress {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this function name.


waitIfCrossShardProcessing(cs, esdtSafeAddrShard, receiverShardId)
chainSim.RequireAccountHasToken(t, cs, getTokenIdentifier(receivedToken), receiver.Bech32, receivedToken.Amount)
if isSftOrMeta(receivedToken.Type) { // expect the contract to have 1 token
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also have an else here?

trnsData := &transferData{
GasLimit: uint64(10000000),
Function: []byte("hello"),
Args: [][]byte{{0x01}},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this contract with 0 numbers if we always call it with non zero value?

func generateAccountsAndTokens(
t *testing.T,
cs chainSim.ChainSimulator,
) map[*chainSim.Account]chainSim.ArgsDepositToken {
Copy link
Contributor

Choose a reason for hiding this comment

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

map[*obj]?
That's really problematic, not ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants