-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update crafters checklist #28
Conversation
* [ ] Check local env | ||
* EIP1559 | ||
* [ ] Run `make estimate` to estimate gas | ||
* [ ] `export ETH_GAS=X` with the output of the command above (e.g. `export ETH_GAS=6_000_000`) |
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.
Gas estimations might not be 100% accurate:
* [ ] `export ETH_GAS=X` with the output of the command above (e.g. `export ETH_GAS=6_000_000`) | |
* [ ] `export ETH_GAS=X` with the output of the command above + a safety margin (e.g. `export ETH_GAS=6_000_000`) |
* EIP1559 | ||
* [ ] Run `make estimate` to estimate gas | ||
* [ ] `export ETH_GAS=X` with the output of the command above (e.g. `export ETH_GAS=6_000_000`) | ||
* [ ] Check current gas price using `seth gas-price` and set `ETH_GAS_PRICE` accordingly (e.g. `50 gwei`) |
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.
Wouldn't it be better to fetch it from the same source as the priority fee?
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.
It might be, but the aim of this PR is to just improve the structure or in some cases add/improve rules, but not change them, so I would put this out of scope 🙂
* [ ] Set `deployed_spell` to `address(0)` | ||
* [ ] Set `deployed_spell_created` to `0` | ||
* [ ] Set `deployed_spell_block` to `0` | ||
* [ ] Consider to add `previous_spell` address IF it haven't been executed yet |
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.
* [ ] Consider to add `previous_spell` address IF it haven't been executed yet | |
* [ ] IF there are outstanding spells that have not been cast yet, add them to `previous_spells`. |
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.
LGTM
Closes #27. A follow up to #18.