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: implement ihrc in hts system contract mocks (#478) #640

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

mshakeg
Copy link
Contributor

@mshakeg mshakeg commented Jan 12, 2024

Description:

This PR extends the IHRC interface solidity version support. Additionally, the HTS System Contract mock now implements the IHRC interface.

Related issue(s):

Fixes #598 and task 5 in #478

Notes for reviewer:

I'm not sure whether IHRC(htsAddress).isAssociated(evmAddress) has yet been implemented, but I've added it here regardless.

Checklist

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

Signed-off-by: Mo Shaikjee <[email protected]>
@mshakeg mshakeg changed the title implement IHRC in HTS system contract mocks feat: implement ihrc in hts system contract mocks Jan 12, 2024
@mshakeg mshakeg changed the title feat: implement ihrc in hts system contract mocks feat: implement ihrc in hts system contract mocks (#478) Jan 12, 2024
@mshakeg
Copy link
Contributor Author

mshakeg commented Jan 12, 2024

Hey @Nana-EC any idea why the PR Title Check still fails?

Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Changes are to a reserved file that should represent released features.
I'd revert the changes the IHRC.sol.
Test addition is fine otherwise

contracts/hts-precompile/IHRC.sol Show resolved Hide resolved
contracts/hts-precompile/IHRC.sol Outdated Show resolved Hide resolved
@Nana-EC
Copy link
Collaborator

Nana-EC commented Jan 12, 2024

Hey @Nana-EC any idea why the PR Title Check still fails?

Not sure.
Can look into it but it's something we can workaround if it's the only issue.

I left other blocking comments

Copy link
Contributor

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

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

LG. leaved some nits suggestions.

contracts/hts-precompile/IHRC.sol Outdated Show resolved Hide resolved
contracts/hts-precompile/IHRC.sol Show resolved Hide resolved
contracts/hts-precompile/IHRC.sol Outdated Show resolved Hide resolved
test/foundry/HederaFungibleToken.t.sol Outdated Show resolved Hide resolved
test/foundry/HederaFungibleToken.t.sol Outdated Show resolved Hide resolved
test/foundry/HederaNonFungibleToken.t.sol Outdated Show resolved Hide resolved
test/foundry/HederaNonFungibleToken.t.sol Outdated Show resolved Hide resolved
@mshakeg mshakeg requested review from AlfredoG87 and Nana-EC January 22, 2024 21:36
Signed-off-by: Mo Shaikjee <[email protected]>
Copy link
Contributor

@ebadiere ebadiere left a comment

Choose a reason for hiding this comment

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

LG.

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

@mshakeg
Copy link
Contributor Author

mshakeg commented Jan 24, 2024

Hey @Nana-EC can we get this merged? don't want to have to resolve any potential conflicts.

@Nana-EC Nana-EC merged commit 8cabc7f into hashgraph:main Jan 24, 2024
20 of 21 checks passed
@Nana-EC Nana-EC added P2 Tooling tooling labels Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Expand Solidity Version Compatibility Interfaces
5 participants