Feat/dirtycheck #73
Feat/dirtycheck #73
Conversation
Now the script accepts --gas-price argument in Gweis, defaulting to 30. Gweis were chosen over weis, because with weis user can accidentally create too cheap transactions which would lead to many pending (and never mined) transactions (most of the tools report gas in Gwei).
Solidity version needs to be updated for https://etherscan.io/verifyContract2.
Now, if the script sees that the repository is not clean (not committed), it does not proceed. Also, it will abort on non-master branches (by default, possible to skip by using ICO_DEPLOY_ANYWAY env), and also it will abort if you haven't pulled for a week. The deployment script is also adding the commit hash to the source verified by Etherscan. Also added crowdsales/*.py to .gitignore for easier tracking of changes.
| @@ -170,7 +171,7 @@ def main(chain, address, token, csv_file, limit, start_from, issuer_address, add | |||
|
|
|||
| transaction = { | |||
| "from": address, | |||
| "gasPrice": int(web3.eth.gasPrice * 1.5) | |||
| "gasPrice": gas_price * 1000000000 | |||
There was a problem hiding this comment.
yep - it all seems good. - ztane have placed some comments already.
But these numbers with a lot of zeros are really hard to read correctly, and they are very critical - I'd suggest creating a global constant of one million - ie - a line in the beggining of the file like MILLION = int(10e6)
and them write these gas_price expressions as: gasprice * 100 * MILLION
( int(10e8) would work, of course, but I find 100 * MILLION more readable)
There was a problem hiding this comment.
I have totally different view on this, even though it's a rather small matter. I would say having constant like MILLION = int(10e6) is not only not correct (10e6 is actually 10**7 => 10 millions) but also doesn't really improve the readability of the code.
The context of this part of code is to convert gas_price from wei to gigawei. So the thing that developers really care about is how many 0 there are, which is the reason why we can say 1000000000 is a hard number to read => easy to make mistake. And for that reason alone it's best to use exponentiation.
And if you want to take it a little bit further, then you can define an utility function like:
def wei_to_gwei(price):
return price * 10 ** 9 # or price * (10 ** 9) if you don't trust your memory on operator precedence
then it's readable, understandable, reusable and testable.
There was a problem hiding this comment.
I would not say it is small matter - and neither that it is "totally different" view. The major problem, of course is reading the zeros. I agree that putting the exponentiation directly will make the number of zeros more apparent (and less prone to subtle catastrophic mistakes as the one I just made, of writting 10e6 thinking about 10 ** 6)
There was a problem hiding this comment.
There is the other matter that int(1e6) is not a compile-time constant, unlike 10 ** 6.
|
|
||
| def git_ismaster(): | ||
| # User can override git_isold checking for a week | ||
| if ((float(os.getenv("ICO_DEPLOY_ANYWAY", 0)) + 604800) > time.time()): |
There was a problem hiding this comment.
As @ztane mentioned, you don't have to use that many parentheses in Python if, here it's perfectly fine to do this:
if float(os.getenv("ICO_DEPLOY_ANYWAY", 0)) + 604800 > time.time():
And if it's me, I would probably move float(os.getenv("ICO_DEPLOY_ANYWAY", 0)) + 604800 to a separate variable before this condition just to make it a little bit easier to understand, something like this:
deploy_anyway = os.getenv("ICO_DEPLOY_ANYWAY", 0) # actually I don't know what this value, should it contain which type?
one_week = 60 * 60 * 24 * 7
now = time.time()
if deploy_anyway + one_week > now:
Btw why is this time check done in this function and not in git_isold()? It doesn't seem to relate to the current function that much.
This adds sanity check on the GIT repository's state, and also some misc stuff which needs to be committed now ( :D )