Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Contract Pipeline Scripts #561
Contract Pipeline Scripts #561
Changes from 2 commits
3939e92
b44012a
c33d0d5
e185384
b0be4a5
7710b45
8f52a3d
986f528
e839ab6
d959f05
d95014c
dbcb26b
e601e3c
9cdb5e9
6c2b688
877239b
4a5e8cc
6d26e31
2da6d99
686a921
95c1d26
44275d7
3d28bf5
1f7d2c4
cca0eef
26da9df
e2d1909
2a4d64f
dfe0ab5
f1d51e0
a5b4845
209a2b7
d4c324a
bbe8cf5
4c27a3c
35ee46f
f1180b3
633e830
2a79014
02aeb82
78a1d31
f4dab18
2c192f9
79d9a2b
6d56eb4
86adea1
ded7606
ff1989b
dca5b1a
f5b3ec6
7eb8e40
8d532f0
b39861f
370f9c2
4cfa884
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just informs the hash of the operation. Right now, we would want these to collide to avoid redundant upgrades, right? Does that take care of everything? In what case would this take a nonce value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually schedules a transaction on the Timelock contract. It performs the call.
It's not feasible for us to include handling the execution of the timelock transaction imo. We'll be using the safe FE anyway, it takes way less time, it populates the transaction for us, handles signature for all the parties involved, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the
0
: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/cae60c595b37b1e7ed7dd50ad0257387ec07c0cf/contracts/governance/TimelockController.sol#L266The delay should be the actual time-lock no? The
value
is just used for the operation hash?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall agreeing on this. This supposed to be dispatched from CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 0 was for value in eth to be transactioned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wdym?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
during deployment you have the option of passing a certain amount of
eth
to subsequent calls because you might want to pay for something. This does not happen here.