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

Create a deploy script in foundry script. #417

Merged
merged 215 commits into from
Jan 31, 2023

Conversation

0xcodercrane
Copy link
Contributor

Resolves #

-Changed DevelopmentDeploy.ts to use  from
-Added USD3CRV_TOKEN as .env value
-array cleanup in soldity scripts
-added prettier-plugin-solidity to yarn lock
@0x4007
Copy link
Member

0x4007 commented Jan 25, 2023

@hashedMae I noticed you replied to many of zgo's comments with "Done"

I suggest "resolving the conversation" as it should reduce clutter and conform to standard git review flow to express that you are done with the task/conversation.

image

@hashedMae
Copy link
Contributor

hashedMae commented Jan 25, 2023 via email

@0x4007
Copy link
Member

0x4007 commented Jan 26, 2023

I can do that, but it was my understanding that it’s poor form for the submitter to mark things as resolved.

On Wed, Jan 25, 2023 at 8:02 AM, アレクサンダー.eth @.> wrote: @.(https://github.com/hashedMae) I noticed you replied to many of zgo's comments with "Done" I suggest "resolving the conversation" as it should reduce clutter and conform to standard git review flow to express that you are done with the task/conversation. — Reply to this email directly, [view it on GitHub](#417 (comment)), or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

I would think the opposite actually. The person working on the PR should have the final say on the conversation. It's happened to me in the past where a team member replied to me on my PR and then resolved the conversation with their suggestion and I missed the suggestion! However if you as the PR developer resolve the conversation, the others can presume that you received the message and implemented any necessary changes.

Copy link
Contributor

@zgorizzo69 zgorizzo69 left a comment

Choose a reason for hiding this comment

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

IMO we should add more explanation about how to deploy locally
especially the anvil script the output is misleading using https://rpc.flashbots.net for anvil... it should be something like forking from RPC_URL also we already have a bunch of url for forking mainnet inside scripts/runner/conf.ts why don't we use them if no RPC-URL or if the URL is not responding (cos yeah anvil script just stop with no error output if rpc is invalid..) ?

packages/contracts/package.json Outdated Show resolved Hide resolved
@hashedMae
Copy link
Contributor

IMO we should add more explanation about how to deploy locally especially the anvil script the output is misleading using https://rpc.flashbots.net for anvil... it should be something like forking from RPC_URL also we already have a bunch of url for forking mainnet inside scripts/runner/conf.ts why don't we use them if no RPC-URL or if the URL is not responding (cos yeah anvil script just stop with no error output if rpc is invalid..) ?

I'd changed it to use the RPC_URL from .env. Using .conf if RPC_URL is undefined is smart though, I'll add that in there.

-Anvil uses the RPC list in conf.ts if RPC_URL in .env is not added
@hashedMae
Copy link
Contributor

Slither is throwing a bunch of errors I'd previously fixed. It looks like these were reintroduced when ABDKMath was removed.

@0x4007
Copy link
Member

0x4007 commented Jan 27, 2023

IMO we should add more explanation about how to deploy locally
especially the anvil script the output is misleading using https://rpc.flashbots.net for anvil... it should be something like forking from RPC_URL also we already have a bunch of url for forking mainnet inside scripts/runner/conf.ts why don't we use them if no RPC-URL or if the URL is not responding (cos yeah anvil script just stop with no error output if rpc is invalid..) ?

@zgorizzo69 perhaps in order to close this PR out faster, you can create a new issue/bounty for this? If you can add any more related context/detail for potentially another bounty hunter to pick it up that will be appreciated.

-ignored return values from IERC20 transfer
-corrected dollarPriceReset and crvPriceReset in Staking
-DollarMintCalculator getDollarsToMint
-UbiquityChef pendingGovernance and _updatePool
-UbiquityChef _updatePool
-contract size exceeds max
@hashedMae hashedMae merged commit ac745d6 into development Jan 31, 2023
@hashedMae hashedMae deleted the feat/406-protocol-deploy-script branch January 31, 2023 19:20
@0x4007 0x4007 mentioned this pull request Feb 7, 2023
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.

Create a deploy script for protocol
4 participants