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.

Sign up for GitHub

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

Jump to bottom

Configuration update for Arbitrum #122

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Configuration update for Arbitrum #122

wants to merge 6 commits into from

Conversation

arsen3d
Copy link
Contributor

@arsen3d arsen3d commented May 24, 2024

Glance - superficial check (from domain experts)

Summary

Deploys LP contracts to local l2 Arbitrum Node, Arbitrum Sepolia, Arbitrum One or Arbitrum Nova

How to test locally

./stack chain-deploy dev_personal
local l2 dev node must be setup
https://docs.arbitrum.io/run-arbitrum-node/run-local-dev-node

@arsen3d arsen3d self-assigned this May 24, 2024
AquiGorka
AquiGorka reviewed May 24, 2024
View reviewed changes
stack Outdated Show resolved Hide resolved
@AquiGorka
Copy link
Contributor

@arsen3d can you please review this document: https://www.notion.so/lilypadnetwork/Git-flow-conventions-98606ed2291b45fba6916dbf6ccab98f, it outlines the naming conventions we should follow for branches and the conventions for git commit messages

@arsen3d
Copy link
Contributor Author

arsen3d commented May 24, 2024 via email

bgins
bgins approved these changes May 25, 2024
View reviewed changes
Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for switching over to using getAccount('admin') 🙏

AquiGorka
AquiGorka suggested changes May 25, 2024
View reviewed changes
Copy link
Contributor

@AquiGorka AquiGorka left a comment •
edited by arsen3d
Loading

Choose a reason for hiding this comment

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

The end goal here is to move our whole system from using geth to using arbitrum in the different environments: local development, devnet (our one node infra) and incentivized tesnet (deployment to arbitrum testnet).

The expectation from this PR is to inlcude all the changes (or point to changes in other repos like faucet) for us to:

  • run things locally in arbitrum instead of geth
  • verify the newly created continuous deployment flow for contracts works for arbitrum
  • if there are changes to the local flow, we need those document in the proper readmes, not just a link to arbitrum's docs.

It is not the end goal here, for you to deploy to Arbitrum, we will plan for that in parallel to the work here.

@bgins bgins mentioned this pull request May 31, 2024
Merged
7 tasks
@arsen3d arsen3d force-pushed the deploy_arbitrum_sepolia branch 2 times, most recently from 71ad575 to 2ef7251 Compare May 31, 2024 22:03
@arsen3d
Copy link
Contributor Author

arsen3d commented May 31, 2024

Glance - superficial check (from domain experts)

Summary

Deploys LP contracts to local l2 Arbitrum Node, Arbitrum Sepolia, Arbitrum One or Arbitrum Nova

How to test locally

./stack chain-deploy dev_personal local l2 dev node must be setup https://docs.arbitrum.io/run-arbitrum-node/run-local-dev-node

@AquiGorka Deployment documentation has been added.

@arsen3d arsen3d force-pushed the deploy_arbitrum_sepolia branch from 7660be1 to d9aa9f1 Compare May 31, 2024 22:09
@galaxyxtwo
Copy link
Contributor

@arsen3d hopefully this is helpful- when using Arbitrum, Ive noticed that tx fees on Arbitrum Nova are typically far lower than on Arb One.

@arsen3d
Copy link
Contributor Author

arsen3d commented Jun 3, 2024

@arsen3d hopefully this is helpful- when using Arbitrum, Ive noticed that tx fees on Arbitrum Nova are typically far lower than on Arb One.

The fee issue was in context of testnet.
But you are right, Nova is a better deal.

bgins
bgins reviewed Jun 5, 2024
View reviewed changes
Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

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

This looks great, nice helpful guide! Left a few initial comments. Will keep testing this once I get funds from a faucet into my wallet.

hardhat/DEPLOYMENT.MD Outdated Show resolved Hide resolved
hardhat/DEPLOYMENT.MD Show resolved Hide resolved
hardhat/DEPLOYMENT.MD Outdated Show resolved Hide resolved
hardhat/DEPLOYMENT.MD Outdated Show resolved Hide resolved
@AquiGorka
Copy link
Contributor

This looks great, nice helpful guide! Left a few initial comments. Will keep testing this once I get funds from a faucet into my wallet.

🙊 https://faucet-testnet.lilypad.tech/

@arsen3d arsen3d force-pushed the deploy_arbitrum_sepolia branch 3 times, most recently from 7a5525c to 021ef23 Compare June 6, 2024 17:27
bgins
bgins reviewed Jun 6, 2024
View reviewed changes
Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Added a few more comments and note about the embedded configuration files. Looking good!

hardhat/DEPLOYMENT.MD Outdated Show resolved Hide resolved
hardhat/DEPLOYMENT.MD Outdated Show resolved Hide resolved
hardhat/DEPLOYMENT.MD Outdated Show resolved Hide resolved
@bgins bgins mentioned this pull request Jun 10, 2024
Merged
2 tasks
@AquiGorka
Copy link
Contributor

@arsen let's rebase and update this PR before merging, there's a couple of outstanding comments to be addressed

@arsen3d arsen3d force-pushed the deploy_arbitrum_sepolia branch 3 times, most recently from fdd5114 to 9c2dd78 Compare June 20, 2024 04:37
@arsen3d arsen3d force-pushed the deploy_arbitrum_sepolia branch from 9c2dd78 to b190c75 Compare June 20, 2024 04:56
@arsen3d arsen3d requested a review from AquiGorka June 20, 2024 05:03
@arsen3d
Copy link
Contributor Author

arsen3d commented Jun 20, 2024

@AquiGorka, not sure what needs to be done to complete this pull request. Does it require you approval?

@AquiGorka
Copy link
Contributor

AquiGorka commented Jun 20, 2024 •
edited
Loading

There's 2 things that have not been addressed:

  • we do not need the configurations forarbitrumOne nor arbitrumNova and the configuration for arbitrumSepolia is not needed as we've already added a testnet configuration.
  • It seems there is a new file: pkg/web3/bindings/pow/pow.go added in this PR which I don't understand how it relates to these changes, can you clarify or remove the file?

With these 2 things addressed I'll approve for us to merge.

@AquiGorka AquiGorka marked this pull request as draft July 11, 2024 16:12
@AquiGorka
Copy link
Contributor

@alvin-reyes this PR aimed at documenting the deploy to arbitrum process, can you also take over here? You can decide whether to use this PR as base or close and open up a new PR with any documentation that should be part of the code base.

@bgins bgins changed the title Configuration update for Arbitrum feat: Configuration update for Arbitrum Jul 24, 2024
@bgins bgins changed the title feat: Configuration update for Arbitrum Configuration update for Arbitrum Jul 24, 2024
@alvin-reyes
Copy link
Contributor

@arsen3d. Re-assigning this to @kquirapas so you can work on the phase 1.2 incentivenet improvement.

@kquirapas
Copy link
Contributor

Added to engineering board assigned to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgins bgins bgins approved these changes

@AquiGorka AquiGorka Awaiting requested review from AquiGorka

Assignees

@arsen3d arsen3d

@alvin-reyes alvin-reyes

@kquirapas kquirapas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@arsen3d @AquiGorka @galaxyxtwo @alvin-reyes @kquirapas @bgins