Conversation
7d43d86 to
13c68e4
Compare
…n an HTS token is created' test Signed-off-by: Denys Sinyakov <[email protected]>
…to the same Smart Wallet and send self-sponsored transactions Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
13c68e4 to
1098d26
Compare
acuarica
left a comment
There was a problem hiding this comment.
Good start, left some comments we can discuss
.gitignore
Outdated
| # Local environment | ||
| .env No newline at end of file | ||
| .env | ||
| node_modules |
There was a problem hiding this comment.
there shouldn't be node_modules in the repo root, we can revert this
| "dev": true, | ||
| "license": "(Apache-2.0 WITH LLVM-exception)" | ||
| }, | ||
| "node_modules/@cspotcode/source-map-support": { |
There was a problem hiding this comment.
why do we have these changes? I don't see any change in package.json
| * @param {number} [gasLimit=1_500_000] - Gas limit | ||
| * @returns {Promise<ethers.TransactionReceipt | null>} | ||
| */ | ||
| async function associateHtsToken(eoa, tokenAddress, nonce, gasLimit = 1_500_000) { |
There was a problem hiding this comment.
maybe HTS specific functions should be placed under its own module, e.g., utils/hts.js, to avoid polluting this web3.js module
There was a problem hiding this comment.
we could rename this to hts.test.js given at the moment contains HTS related tests.
Additional Hiero specific tests could be placed in their own file.
| @@ -0,0 +1,138 @@ | |||
| const assert = require('node:assert').strict; | |||
| const log = require('node:util').debuglog('hip-1340'); | |||
There was a problem hiding this comment.
each module should have its own logger section, e.g.,
| const log = require('node:util').debuglog('hip-1340'); | |
| const log = require('node:util').debuglog('hip-1340:hts'); |
| await waitFor(tokenCreateContract.grantTokenKycPublic(tokenAddress, receiver.address)); | ||
|
|
||
| // Transfer HTS tokens from treasury to both EOAs | ||
| await waitFor(tokenCreateContract.transferTokenPublic(tokenAddress, eoa1.address, 5_000)); |
There was a problem hiding this comment.
nit: all amounts, either HBAR or token's should be bigints for consistency
| await waitFor(tokenCreateContract.transferTokenPublic(tokenAddress, eoa1.address, 5_000)); | |
| await waitFor(tokenCreateContract.transferTokenPublic(tokenAddress, eoa1.address, 5_000n)); |
| provider = (await ethers.getSigners())[0].provider; | ||
| network = await provider.getNetwork(); | ||
|
|
||
| return { provider, network, log }; |
There was a problem hiding this comment.
if log is not used, we can remove it
There was a problem hiding this comment.
Good catch, I used to use it, but no longer the case
| // Deploy Simple7702Account (the Smart Wallet both EOAs will delegate to) | ||
| const smartWallet = await deploy(SIMPLE_7702_ACCOUNT); | ||
|
|
||
| const [eoa1Nonce, eoa2Nonce, receiverNonce] = [new Nonce(), new Nonce(), new Nonce()]; |
There was a problem hiding this comment.
Given that eoa1Nonce, et. all refers to the same nonce (no aliases) we could use Number instead, right?
There was a problem hiding this comment.
| const anotherAddressBalance = await tokenContract.balanceOf(anotherAddress.address); | ||
| expect(anotherAddressBalance).to.be.equal(1000); | ||
|
|
||
| // Verify the HTS token address has a delegation designator pointing to 0x167 |
There was a problem hiding this comment.
At the moment it can be verified by checking both ContractByteCodeQuery and AccountInfo.delegationAddress from CN, something like
const [code, contractBytecode, delegationAddress] = await web3.getCodes(eoa.address);
// TODO(pectra): Reenable check once MN and Relay include support for EIP-7702
// expect(code).to.be.equal(designatorFor(delegateAddress));
expect(contractBytecode).to.be.equal(designatorFor(delegateToAddress));
expect(delegationAddress).to.be.equal(delegateToAddress);There was a problem hiding this comment.
const [code, contractBytecode, delegationAddress] = await web3.getCodes(eoa.address);Does not seem to work. In my case that won't be eoa, but contract address. And we call mirror node accounts endpoint underneath. I tried using contracts endpoint instead, but mirror node did not return any info. Will look further, to see how that code can be adapted for this case.
|
|
||
| it('should return delegation designation to 0x167 when an HTS token is created', async function () { | ||
| // Deploy TokenCreateContract and create HTS fungible token | ||
| const tokenCreateContract = await Utils.deployTokenCreateContract(); |
There was a problem hiding this comment.
why do we need to deploy a token contract here?
There was a problem hiding this comment.
One way to create HTS is to use the SDK. Another way to do it is via this helper contract that is used in other test. This way we are doing it via EVM only (no native/SDK).
There was a problem hiding this comment.
Oh I see, so then maybe we can parameterize this test by using both EVM and SDK HTS tokens.
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
|
@acuarica addressed most comments. Looking further into why below suggestion did not work as expected const [code, contractBytecode, delegationAddress] = await web3.getCodes(eoa.address);
// TODO(pectra): Reenable check once MN and Relay include support for EIP-7702
// expect(code).to.be.equal(designatorFor(delegateAddress));
expect(contractBytecode).to.be.equal(designatorFor(delegateToAddress));
expect(delegationAddress).to.be.equal(delegateToAddress); |
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Fixed this, by querying the |
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
fe7fb24 to
82518b6
Compare
Signed-off-by: Denys Sinyakov <[email protected]>
…a-evm-testing into denys/run-pectra-tests-with-hh
stoyanov-st
left a comment
There was a problem hiding this comment.
The test build is failing due to the targeted CN branch being deleted after a PR was merged.
Please change it to feature/hedera-evm-pectra-support
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
…/hedera-evm-testing into denys/run-pectra-tests-with-hh
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
…a-evm-testing into denys/run-pectra-tests-with-hh
Signed-off-by: Denys Sinyakov <[email protected]>
Signed-off-by: Denys Sinyakov <[email protected]>
Description:
This PR add several Pectra test case related to Hiero functionality.
Related issue(s):
Fixes #
Notes for reviewer:
Checklist