add updated deploys #350
add updated deploys #350
Conversation
| "transactionType": "CREATE", | ||
| "contractName": "PoolManager", | ||
| "contractAddress": "0xf242ce588b030d0895c51c0730f2368680f80644", | ||
| "contractAddress": "0xe8e23e97fa135823143d6b9cba9c699040d51f70", |
There was a problem hiding this comment.
i feel like ive said this every time... but imo we should never be pushing run-latest to github as its a file that gets updated regularly. It would be better to put run-timestamp to the repo so we can keep track of all deployments through time
There was a problem hiding this comment.
We discussed this on the previous merge and opted to do the opposite. Theres not really a benefit in knowing any of the old contracts... esp when we're changing interfaces so much.
There was a problem hiding this comment.
Also you can just look at the github history if you rlly wanna know...
There was a problem hiding this comment.
ok... i wasnt part of that discussion. I still disagree, we regularly get questions that are like "is X an official uniswap deploy?" and i think it would be much easier to search to confirm/deny going forwards if all of the addresses we've deployed over time are tracked in the repo. That way we can easily say "yes but thats an outdated deploy" or "no" or "yes thats our latest deploy".
There was a problem hiding this comment.
🤷♀️ I feel like this is completely sufficient for our use case tbh which is to just track the latest address. We would never point an integrator to an old version of v4 especially since the interfaces continue to change. I do agree that is a question we get but usually its in regard to canonical deploys on other chains.
There was a problem hiding this comment.
If you feel really strongly I can change it to keep the old ones - Just leave one last comment to signal which way then
There was a problem hiding this comment.
Hey all. Would really appreciate having timestamped run-latest. I'm developing Uniswap hooks during a hackathon and the deployments in general seem all over the place sometimes haha. Just an input from my side, cheers!
There was a problem hiding this comment.
@neelansh15 Thats great feedback! We will soon have a better and more consistent record for deploys
| @@ -18,7 +18,7 @@ contract DeployPosmTest is Script { | |||
| { | |||
| vm.startBroadcast(); | |||
|
|
|||
| posm = new PositionManager{salt: hex"03"}( | |||
| posm = new PositionManager{salt: hex"01"}( | |||
There was a problem hiding this comment.
why not just use create1 to stop needing to change the salt every time?
There was a problem hiding this comment.
We actually usually don't need to change the salts each time bc the bytecode is so different from deploy to deploy. On posm I started using salt when I was testing to see how different ascii look on etherscan 😄
There was a problem hiding this comment.
I dont wanna change that though bc we've deployed the current deploys with create2 so nice to have a record in this PR what salt it was deployed with
Related Issue
Which issue does this pull request resolve?
Description of changes