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: test that deploy offchain in the layer 2. #53

Closed
wants to merge 5 commits into from

Conversation

LeonardoVieira1630
Copy link
Member

@LeonardoVieira1630 LeonardoVieira1630 commented Apr 4, 2024

feat: test that deploys offchain in the layer 2

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Description

Pull request will add an other test file that deploys the contracts in the Arbitrum local node. This way it will be possibly to test the complete flow of interactions including l2 contracts.

Related Issue

#51
#47

Changes

  • New feature implementation
  • Bug fix
  • Code refactoring
  • Documentation update
  • Other (please specify)

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

packages/client/README Outdated Show resolved Hide resolved
packages/client/README Outdated Show resolved Hide resolved

4. With the local Hardhat node running, execute the E2E tests:
````cmd
yarn client hardhat test ./test/l2_e2e.spec.ts --network localhost
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be a script on the client's package.json as well.

perhaps even following this example

Copy link
Member Author

Choose a reason for hiding this comment

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

I tryed to use a script, but it was not possible to run the node and the tests in parallel. Maybe we can work on this improviment later.




# Running Tests with Local Arbitrum Node
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Running Tests with Local Arbitrum Node
## Running Tests with Local Arbitrum Node

packages/client/README Show resolved Hide resolved
import { localhost } from 'viem/chains'
import { createTestClient, http, publicActions } from 'viem'
import { HardhatEthersSigner } from '@nomicfoundation/hardhat-ethers/signers'
// import { expect } from 'chai'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// import { expect } from 'chai'

Copy link
Member Author

Choose a reason for hiding this comment

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

It will be used after the gatway_l2 implementation. That is why it was not removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this file could be called arbitrum_e2e.spec.ts since we might want to integrate other L2's in the near future

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice call.

packages/client/test/l2_e2e.spec.ts Outdated Show resolved Hide resolved
Comment on lines 52 to 98
async function deployOffchainResolver(): Promise<void> {
const walletForL2 = new Wallet(devAccountPrivateKey)
const l2Signer = walletForL2.connect(l2Provider)

const ResolverContract = await new eth.ContractFactory(
abiOffchainResolver,
bytecodeOffchainResolver,
l2Signer,
).deploy(gatewayUrl, signers)

offchainResolver = await eth.getContractAt(
abiOffchainResolver,
await ResolverContract.getAddress(),
)
console.log('Offchain resolver: ', await ResolverContract.getAddress())
}

// Function to deploy registry contract
async function deployRegistry(): Promise<void> {
const l1Signer = await l1Provider.getSigner(devAccountPublicKey)
const RegistryContract = await new eth.ContractFactory(
abiRegistry,
bytecodeRegistry,
l1Signer,
).deploy()

registry = await eth.getContractAt(
abiRegistry,
await RegistryContract.getAddress(),
)
console.log('Registry: ', await RegistryContract.getAddress())

await (registry.connect(l1Signer) as Contract).setSubnodeRecord(
root,
labelhash('eth'),
l1Signer,
await offchainResolver.getAddress(),
10000000,
)
await (registry.connect(l1Signer) as Contract).setSubnodeRecord(
namehash('eth'),
labelhash('offchain'),
l1Signer,
await offchainResolver.getAddress(),
10000000,
)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be shared with the other e2e spec file

Comment on lines 146 to 157
// console.log('ok!')
// try {
// await client.getEnsAvatar({
// name: ensAddress,
// universalResolverAddress:
// (await UniversalResolverContract.getAddress()) as `0x${string}`,
// })
// } catch (error) {
// const customError = error as BaseError
// console.log(customError.message)
// // expect(customError.message).contain('HTTP request failed')
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

is this commented out on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will be used after the gatway_l2 implementation. That is why it was not removed.

@pikonha
Copy link
Contributor

pikonha commented Apr 8, 2024

the client's package.json is still using vitest for running the tests

@@ -2,7 +2,7 @@
"name": "@blockful/client",
"packageManager": "yarn@4.1.0",
"scripts": {
"test": "vitest run",
"test": "yarn hardhat test ./test/arbitrum_e2e.spec.ts --network localhost",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"test": "yarn hardhat test ./test/arbitrum_e2e.spec.ts --network localhost",
"test:arb": "yarn hardhat test ./test/arbitrum_e2e.spec.ts --network localhost",

@@ -2,7 +2,7 @@
"name": "@blockful/client",
"packageManager": "yarn@4.1.0",
"scripts": {
"test": "vitest run",
"test": "yarn hardhat test ./test/arbitrum_e2e.spec.ts --network localhost",
"test:watch": "vitest",
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this line

Copy link
Contributor

Choose a reason for hiding this comment

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

rename this file to deploys.ts

import { normalize } from 'viem/ens'
import { localhost } from 'viem/chains'
import { createTestClient, http, publicActions } from 'viem'
const gatewayUrl = 'http://127.0.0.1:3000'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const gatewayUrl = 'http://127.0.0.1:3000'
const gatewayUrl = 'http://127.0.0.1:3000/{sender}/{data}.json'

before(async () => {
const signers = await eth.getSigners()

const walletForL2 = new Wallet(devAccountPrivateKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

const gatewayUrl = 'http://127.0.0.1:3000'

// Providers
const l2Provider = new eth.JsonRpcProvider('http://127.0.0.1:8547')
Copy link
Contributor

Choose a reason for hiding this comment

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

const offchainResolverContract = await deployOffchainResolver(
l2Signer,
gatewayUrl,
signers as unknown as string,
Copy link
Contributor

Choose a reason for hiding this comment

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

double-check this conversion

because the offchain part is not implemented yet.
*/
it('Call ENS flow with viem.', async () => {
// ENS address
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// ENS address


// Deploying the contracts
const offchainResolverContract = await deployOffchainResolver(
l2Signer,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be deployed on L1 and the PublicResolver on the L2

@pikonha pikonha marked this pull request as draft April 9, 2024 13:36
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.

None yet

2 participants