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: Resolve IHRC632 and IHRC904 definitions #1103

Merged
merged 11 commits into from
Dec 13, 2024
Merged

Conversation

Nana-EC
Copy link
Collaborator

@Nana-EC Nana-EC commented Dec 13, 2024

Description:
The repo had been missing specs for HIP 632 and 904 and 906.
Additionally, there wasn't a clear separation of facade contracts that EOAs can call directly from calls that a contract would use that would allow it to specify the entity being modified.

Each system contract HIP introduced new functions that apply to IHederaTokenService and IHederaAccountService.
For clearer specification an IHRCXXXX contract for a specific HIP XXXX will contain the functions introduced by that HIP that are callable by a contract only and utilize the system contract.

Facade contracts will contain functions that are callable by contracts and EOAs directly.
In this case the address is the address of the entity and not the system contract.

The interfaces are introduced in this PR but eventually the goal is to let the System contract interfaces inherit the non facade interfaces that describe it e.g. IHederaAccountService is IHRC632, IHRC906

  • Correct IHRC632.sol with rename to IHRC906 and add comments
  • Add IHRC906Facade.sol w hbarAllowance() and hbarApprove()
  • Add getEvmAddressAlias(), getHederaAccountNumAlias(), isValidAlias(), isAuthorizedRaw() and isAuthorized() to IHRC632.sol
  • Add IHRC904.sol
  • Add IHRC904TokenFacade.sol
  • Add IHRC904AccountFacade.sol
  • Add isAuthorized() to HederaAccountService.sol
  • Add an IHTSStructs.sol that contains all the structs used by the system contract and makes it easy for function reuse
  • Add ABI files

Related issue(s):

Fixes #1083
Fixes #1101
Fixes #1102

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@Nana-EC Nana-EC requested a review from quiet-node December 13, 2024 03:46
@Nana-EC Nana-EC self-assigned this Dec 13, 2024
@Nana-EC Nana-EC changed the title Resolve IHRC632 and IHRC904 definitions feat: Resolve IHRC632 and IHRC904 definitions Dec 13, 2024
Copy link

github-actions bot commented Dec 13, 2024

Test Results

 16 files  + 1   83 suites  +7   12m 4s ⏱️ +20s
294 tests +14  288 ✅ +12  6 💤 +3  0 ❌  - 1 
303 runs  +23  296 ✅ +20  7 💤 +4  0 ❌  - 1 

Results for commit eb73397. ± Comparison against base commit 364927f.

This pull request removes 3 and adds 17 tests. Note that renamed tests count towards both.
Should allow owner to grant an allowance to spender using IHRC632 and spender to transfer allowance to receiver on behalf of owner ‑ @CryptoAllowance Test Suite Should allow owner to grant an allowance to spender using IHRC632 and spender to transfer allowance to receiver on behalf of owner
should execute hbarAllowance() by an EOA to retrieve allowance granted to a spender ‑ @IHRC-632 @CryptoAllowance  Test Suite should execute hbarAllowance() by an EOA to retrieve allowance granted to a spender
should execute hbarApprove() by an EOA to grant an hbar allowance to another EOA ‑ @IHRC-632 @CryptoAllowance  Test Suite should execute hbarApprove() by an EOA to grant an hbar allowance to another EOA
Should allow owner to grant an allowance to spender using IHRC906AccountFacade and spender to transfer allowance to receiver on behalf of owner ‑ @CryptoAllowance Test Suite Should allow owner to grant an allowance to spender using IHRC906AccountFacade and spender to transfer allowance to receiver on behalf of owner
should be able to associate() to the token from a contract ‑ @HRC-719 Test Suite HRC719 wrapper contract should be able to associate() to the token from a contract
should be able to associate() to the token from an EOA ‑ @HRC-719 Test Suite HRC719 Token should be able to associate() to the token from an EOA
should be able to call isAssociated() after token association ‑ @HRC-719 Test Suite HRC719 wrapper contract should be able to call isAssociated() after token association
should be able to call isAssociated() after token dissociation ‑ @HRC-719 Test Suite HRC719 wrapper contract should be able to call isAssociated() after token dissociation
should be able to call isAssociated() to the token from an EOA when associated ‑ @HRC-719 Test Suite HRC719 Token should be able to call isAssociated() to the token from an EOA when associated
should be able to call isAssociated() to the token from an EOA when dissociated ‑ @HRC-719 Test Suite HRC719 Token should be able to call isAssociated() to the token from an EOA when dissociated
should be able to call isAssociated() to the token from an EOA ‑ @HRC-719 Test Suite HRC719 Token should be able to call isAssociated() to the token from an EOA
should be able to call isAssociated() ‑ @HRC-719 Test Suite HRC719 wrapper contract should be able to call isAssociated()
should be able to dissociate() to the token from an EOA ‑ @HRC-719 Test Suite HRC719 Token should be able to dissociate() to the token from an EOA
…
This pull request removes 1 skipped test and adds 4 skipped tests. Note that renamed tests count towards both.
should execute hbarAllowance() by an EOA to retrieve allowance granted to a spender ‑ @IHRC-632 @CryptoAllowance  Test Suite should execute hbarAllowance() by an EOA to retrieve allowance granted to a spender
should be able to call isAssociated() to the token from an EOA when associated ‑ @HRC-719 Test Suite HRC719 Token should be able to call isAssociated() to the token from an EOA when associated
should be able to call isAssociated() to the token from an EOA when dissociated ‑ @HRC-719 Test Suite HRC719 Token should be able to call isAssociated() to the token from an EOA when dissociated
should be able to call isAssociated() to the token from an EOA ‑ @HRC-719 Test Suite HRC719 Token should be able to call isAssociated() to the token from an EOA
should execute hbarAllowance() by an EOA to retrieve allowance granted to a spender ‑ @IHRC-906 Facade @CryptoAllowance  Test Suite should execute hbarAllowance() by an EOA to retrieve allowance granted to a spender

♻️ This comment has been updated with latest results.

@Nana-EC Nana-EC added the enhancement New feature or request label Dec 13, 2024
@Nana-EC Nana-EC added this to the 0.11.0 milestone Dec 13, 2024
Nana-EC and others added 8 commits December 12, 2024 23:28
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
This reverts commit 54bb0e6.
Signed-off-by: Logan Nguyen <[email protected]>
@Nana-EC Nana-EC marked this pull request as ready for review December 13, 2024 05:43
acuarica
acuarica previously approved these changes Dec 13, 2024
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

lg, left some questions

quiet-node
quiet-node previously approved these changes Dec 13, 2024
@Nana-EC Nana-EC dismissed stale reviews from quiet-node and acuarica via 2ef3170 December 13, 2024 06:38
acuarica
acuarica previously approved these changes Dec 13, 2024
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

lg

Signed-off-by: Nana Essilfie-Conduah <[email protected]>
@Nana-EC Nana-EC merged commit c59a96d into main Dec 13, 2024
30 checks passed
@Nana-EC Nana-EC deleted the 1083-update-i632-sol branch December 13, 2024 07:04
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
None yet
Development

Successfully merging this pull request may close these issues.

Add an IHRC904Facade.sol Add an IHRC904.sol that captures functions Update IHRC632 interface
3 participants