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

feat: add new tests for HIP-1010 #911

Merged
merged 15 commits into from
Oct 1, 2024

Conversation

konstantinabl
Copy link
Contributor

@konstantinabl konstantinabl commented Aug 19, 2024

Description:

Testing new functionality regarding fees, namely updateFungibleTokenCustomFeesPublic and updateNonFungibleTokenCustomFeesPublic (HIP-1010), but ALSO testing if fees are correctly deducted from accounts, since there were no such test before. Thus, the tests in this PR are slightly bigger and more complex than expected

Related issue(s):

Fixes #876

@konstantinabl konstantinabl linked an issue Aug 19, 2024 that may be closed by this pull request
@konstantinabl konstantinabl added the enhancement New feature or request label Aug 19, 2024
Copy link

github-actions bot commented Aug 19, 2024

Test Results

 16 files  + 1   82 suites  +10   12m 8s ⏱️ + 7m 30s
290 tests +81  284 ✅ +85  6 💤 ±0  0 ❌  - 4 
299 runs  +53  292 ✅ +56  7 💤 +1  0 ❌  - 4 

Results for commit c1a509b. ± Comparison against base commit 40d2824.

This pull request removes 3 and adds 84 tests. Note that renamed tests count towards both.
"before all" hook for "should be able to delete token" ‑ TokenManagmentContract Test Suite "before all" hook for "should be able to delete token"
"before all" hook for "should be able to increase and decrease counter on V1" ‑ Proxy Upgrade Contracts Test Suite Counter Upgradable Contract Test Suite "before all" hook for "should be able to increase and decrease counter on V1"
"before each" hook for "Should be able to mint a new token" ‑ @OZERC1155Token Test Suite "before each" hook for "Should be able to mint a new token"
Should NOT allow a non-operator to transfer tokens to another account ‑ @OZERC1155Token Test Suite Should NOT allow a non-operator to transfer tokens to another account
Should NOT allow a spender to spend hbar on behalf of owner without an allowance grant ‑ @CryptoAllowance Test Suite Should NOT allow a spender to spend hbar on behalf of owner without an allowance grant
Should NOT allow an approval on behalf of hbar owner WITHOUT its signature ‑ @CryptoAllowance Test Suite Should NOT allow an approval on behalf of hbar owner WITHOUT its signature
Should NOT burn insufficient amount of token ‑ @OZERC1155Token Test Suite Should NOT burn insufficient amount of token
Should NOT transfer the ownership to another account if the caller is not owner ‑ @OZERC1155Token Test Suite Should NOT transfer the ownership to another account if the caller is not owner
Should allow a crypto owner contract account to grant an allowance to a spender contract account to transfer allowance to a receiver on behalf of owner contract account ‑ @CryptoAllowance Test Suite Should allow a crypto owner contract account to grant an allowance to a spender contract account to transfer allowance to a receiver on behalf of owner contract account
Should allow an approval on behalf of hbar owner WITH its signature ‑ @CryptoAllowance Test Suite Should allow an approval on behalf of hbar owner WITH its signature
Should allow an operator to transfer a token to another account ‑ @OZERC1155Token Test Suite Should allow an operator to transfer a token to another account
Should allow an operator to transfer tokens in batch to another account ‑ @OZERC1155Token Test Suite Should allow an operator to transfer tokens in batch to another account
Should be able to mint a new token ‑ @OZERC1155Token Test Suite Should be able to mint a new token
…

♻️ This comment has been updated with latest results.

@konstantinabl
Copy link
Contributor Author

Blocked by hashgraph/hedera-mirror-node#9136
We need this change in the relay in order to be able to return predefined gas for smart contracts that are in services, but not yet in the mirror node

@konstantinabl
Copy link
Contributor Author

This issue was resolved, however currently waiting for a new release of the local node that will use newest mirror node tag 0.113.1

Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
@konstantinabl konstantinabl force-pushed the 876-hip-1010-solidity-reference-definition branch from f39602c to b2326fc Compare September 20, 2024 15:23
@konstantinabl konstantinabl marked this pull request as ready for review September 20, 2024 16:11
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
@konstantinabl konstantinabl added this to the 0.10.0 milestone Sep 30, 2024
@konstantinabl konstantinabl force-pushed the 876-hip-1010-solidity-reference-definition branch from 17b41a9 to c1a509b Compare September 30, 2024 11:09
Copy link
Contributor

@natanasow natanasow left a comment

Choose a reason for hiding this comment

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

LGTM, some nits/questions 🚀

@@ -395,6 +494,64 @@ class Utils {
return tokenAddress;
}

static hexToASCII(str) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a similar method defined here https://github.com/hashgraph/hedera-smart-contracts/blob/main/test/precompile/hedera-token-service/redirect-for-token.js#L40. We may consider extracting these methods in utils class?

@@ -577,6 +745,18 @@ class Utils {
}
}

static async getAccountBalance(address) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's for test purposes only, but we can use the mirror node for balance querying, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@natanasow yes, we can

@konstantinabl konstantinabl merged commit 485c6a7 into main Oct 1, 2024
32 of 33 checks passed
@konstantinabl konstantinabl deleted the 876-hip-1010-solidity-reference-definition branch October 1, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

HIP-1010: Solidity reference definition
2 participants