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

Add deploy multi chain rust script #1021

Open
wants to merge 34 commits into
base: staging
Choose a base branch
from

Conversation

andreivladbrg
Copy link
Member

@andreivladbrg andreivladbrg commented Aug 19, 2024

Closes #971

This PR re-implements the logic from the .sh script into a Rust script to make it more robust and maintainable.

New features not mentioned in the issue:

  • Implements Solidity to write the deployed contracts in the docs format, allowing for easy copy-pasting.
  • Move the admin multisig declarations in Soldity
  • Automatically reads all configured chains from foundry.toml.
  • Automates the process of moving the broadcast files to v2-deployments.

Usage

  1. cd deploy-multi-chain
  2. Examples:
    • cargo run - Defaults to Sepolia and simulates the deployment.
    • cargo run -- --all - Simulates the deployment to all chains found in the foundry.toml file.
    • cargo run -- --all --broadcast - Deploys to all chains found in the foundry.toml file.
    • cargo run -- --cp-bf - Simulates on Sepolia and moves the broadcasted JSON files generated by Foundry to the v2-deployments directory at the path: "../../v2-deployments/protocol/v{}/broadcasts/{}.json". If the dir doesn’t exist, it will be automatically created.

Notes

  • The paths declared (e.g. "../foundry.toml") in the Rust script are relative to the directory from which the script is run.
    • In the Usage section, I mentioned the first step is to use "cd", as it is a requirement to run cargo run from the root of the deploy-multi-chain dir
    • We can change the paths to run it as a CLI standalone with a command like deploy-multi-chain --all, but this will require installing it with cargo install --path . - this is up for debate.
  • I have used the current timestamp to generate always an unique file name in case it already exists (ref point 3 in the issue)
  • I have not added styling to the script output, as it may change.

PaulRBerg and others added 29 commits July 26, 2024 12:43
* docs: status temperature

* Update src/types/DataTypes.sol

Co-authored-by: smol-ninja <[email protected]>

---------

Co-authored-by: smol-ninja <[email protected]>
* refactor: reorder MAX_BROKER_FEE

* refactor: prioritise constants in interface

---------

Co-authored-by: smol-ninja <[email protected]>
feat: add core dir in src
refactor: remove redundant imports

Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: smol-ninja <[email protected]>
Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: smol-ninja <[email protected]>
refactor: precompile constants

test: merge Base_Test contracts for both core and periphery
test: add Periphery_Test contract

test: add params for create with null broker

test: use broker null param functions in batch

test: update the starting timestamp

test: use recipient0 in core tests

test: fix periphery tests

test: use recipient0 in core tests
test: add an init merkle tree function
test: add tranches functions for merkle lockup

test: remove unneeded brokerFee declarations

test(periphery-fork): use null broker in batch

test(fork): make the admin the caller to fix merkle tests

test: add caller param in computeMerkleAddress function

refactor: remove core and periphery remappings

test: replace block.timestamp with getBlockTimestamp
test: remove unused imports

Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
feat: add core and periphery dirs under script
feat: implement DeployDeterministicProtocol script

refactor(shell): update CLI run in generate-svg script

refactor(script): use count maps in deploy protocol

chore: re-order imports

Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: smol-ninja <[email protected]>
ci: use JSON inputs in generate svg workflow

ci: lower the fuzz runs to 2000

ci: remove periphery path where not needed

chore: fix use of fromJSON

Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: smol-ninja <[email protected]>
refactor(benchmark): use users.recipient0
refactor(benchmark): dry-fy params return
chore(benchmark): run benchmark to update results

Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: smol-ninja <[email protected]>
build(shell): use deploy protocol deployment script

Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: smol-ninja <[email protected]>
test: rename recipient0 to recipient
test: refactor variables in Defaults
test(fork): allow leadData to have length 1

Co-authored-by: PaulRBerg <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>

* refactor: remove V2 from codebase
refactor: refer to Lockup contracts

* fix: make NFTDescriptor compatible with previous versions

* fix: use sablier as variabel name in tokenURI

* chore: update precompiles

* refactor: prefix Sablier in NFT mapsymbol

* refactor(periphery): order alphabetically functions in batch interfaces

style: center headers
style: improve the asci art
style: remove empty spaces
chore: add todos

* test: computing merkle proof for 1 leaf

* refactor: use lockup as variable in NFT Descriptor

* refactor: replace v2-core with lockup in urls

* test: add v1.2.0 to NFTDescriptor_Fork_Test
refactor: use package versions in NFTDescriptor_Fork_Test

* refactor: rename SablierNFTDescriptor to LockupNFTDescriptor

* docs(README): remove periphery reference

* test: add TODO over loadDependencies function

---------

Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
* feat: add SablierMerkleInstant

refactor: abstract contract SablierMerkleLockup to SablierMerkleBase
refactor: library MerkleLockup to MerkleBase
refactor: contract MerkleLockupFactory to MerkleFactory

test: add test for MerkleInstant contract
chore: add CreateMerkleInstant script
test: use super.setUp() where setUp() is redundant
test: rename merkle-lockup folder to merkle
test: rename MerkleLockup_Integration_Test contract to Merkle_Shared_Integration_Test
test: fix event assersion in merkleLL and merkleLT

refactor: rename MerkleLockup to Merkle Lockup in NatSpecs

Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>

* test: include asset transfer log in merkle instant claims

* test(refactor): rename Merkle_Shared_Integration_Test back to Merkle_Integration_Test

* test: refactor merkle-lockup folder to merkle-campaign

Signed-off-by: smol-ninja <[email protected]>

* refactor: polish code for createMerkleLL and createMerkleLT scripts
refactor: rename DeployMerkleLockupFactory to DeployMerkleFactory

Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>

* test(fork): remove unneeded vars

---------

Signed-off-by: smol-ninja <[email protected]>
Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
test: polish shared tests
* docs: polish wording in NatSpec

* docs: fixes
* refactor: dry-fy claim function

test: update tests accordingly

test: dry fy claim tests

test: finish claim tests

test: fixes and polishes

refactor: remove unneeded bitmap import

test: move helper claim function

build: include benchmark dir in lint:sol

chore: remove unused imports

test: move base merkle contract

* style: format using forge formatter

* Address Shub's feedback

* test: make the shared claim virtual

* test: inherit the campaign shared test contract in claim

* test: claim polishes

Co-authored-by: smol-ninja <[email protected]>

---------

Co-authored-by: smol-ninja <[email protected]>
* feat: allow end time to be in the past

* test(fork): check isSettled value prior to assertions
feat(script): add an admin map in base script
feat(script): implement getVersion function
refactor(script): move protocol scripts in their dir
refactor(script): move protocol admin map
refactor(script): dry-fy the code
fix: update factory name
@andreivladbrg andreivladbrg marked this pull request as draft August 19, 2024 11:41
feat: create the deployments if it doesn't exists
feat: backup previous deployments files using the timestamp
Copy link
Member

@PaulRBerg PaulRBerg left a comment

Choose a reason for hiding this comment

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

Shouldn't this script live in its own dedicated repository?

  1. We might want to use it in other Solidity repos, e.g. Flow
  2. Rust code requires a dependency management solution (i.e. Cargo) and a formatter (RustFmt)

Check out my rust-template.

fix: handle non-set nameMap for unknown chain id
@andreivladbrg andreivladbrg marked this pull request as ready for review August 22, 2024 10:58
@andreivladbrg
Copy link
Member Author

andreivladbrg commented Aug 22, 2024

Shouldn't this script live in its own dedicated repository?

it doesn’t have the “bandwidth” to have its own dedicated repository. it’s built specifically to run the DeployProtocol or DeployDeterministicProtocol scripts, at their specific paths (e.g. ../script/protocol/DeployProtocol.s.sol).

creating a project of this nature requires more time and full focus. this is actually what sphinx (if you remember them) wanted to build, but they didn’t succeed.

also, i wouldn’t have built it in rust, as it isn’t the friendliest for these kinds of tasks :)) i would have chosen something lighter, like js or python

Rust code requires a dependency management solution (i.e. Cargo) and a formatter (RustFmt)

lol, ik :))

@andreivladbrg
Copy link
Member Author

andreivladbrg commented Aug 22, 2024

The PR is ready to be reviewed, @PaulRBerg @smol-ninja.

The code is more concise compared to the .sh script, but if you think the switch to Rust isn’t justified, I can understand. I built it as a starting project to learn more about the language itself.

@andreivladbrg andreivladbrg linked an issue Aug 22, 2024 that may be closed by this pull request
@PaulRBerg
Copy link
Member

it doesn’t have the “bandwidth” to have its own dedicated repository.

I think it does. It seems like a bad practice to mix two programming paradigms (Solidity and Rust) in the same repository.

creating a project of this nature requires more time and full focus

You can keep the code as is. All you would need to do in a separate repo is use git submodules to import the Solidity code.

lol, ik :))

Sorry, I missed the Cargo.toml. However, this PR does not introduce a rustfmt.toml file.

@andreivladbrg
Copy link
Member Author

I think it does. It seems like a bad practice to mix two programming paradigms (Solidity and Rust) in the same repository.

i honestly don’t think it is bad practice to mix multiple programming languages in a single repo, especially since Solidity is a very specific language meant for implementing Ethereum smart contracts. as our codebase becomes more complex, it’s not feasible to use just one language for every task.

You can keep the code as is. All you would need to do in a separate repo is use git submodules to import the Solidity code.

i think i have an idea - we don’t need to use git submodules, as we can just install it as a standalone CLI that is meant to be used in the root of this repo. please refer to my first note in the OP

@PaulRBerg
Copy link
Member

honestly don’t think it is bad practice to mix multiple programming languages in a single repo

I'm still against it, but OK, let's see how it works with the Rust code in this repo.

as we can just install it as a standalone CLI

In that case, I'd move it to a separate repo. That's where standalone stuff should be.

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.

Improve the deploy multi chain script
3 participants