-
Notifications
You must be signed in to change notification settings - Fork 66
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
bump Vault #704
bump Vault #704
Conversation
execute on PR execute on PR execute on PR execute on PR execute on PR use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image use the correct machine image own the auth file own the auth file nodejs legacy rem node-gyp install
13b382a
to
065d391
Compare
@@ -96,12 +105,11 @@ jobs: | |||
name: Apply Overrides | |||
command: make init_multisig | |||
working_directory: ~/repo/MultiSigWalletOverride | |||
# node symlink missing if there's not nodejs-legacy | |||
- run: sudo apt-get update && sudo apt-get install -y build-essential libhidapi-dev libudev-dev libusb-1.0-0 libusb-1.0-0-dev nodejs-legacy |
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.
what's happening to the original nvm installation?
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 intial problem was that ubuntu 14 (which is what cirleci uses if you don't define an image) uses a really old gcloud and docker and docker compose. that didn't allow me to authenticate towards our GCS registry. so I had to bump to ubuntu 16. image: ubuntu-1604:201903-01
now that broke helluva lot of things with gnosis 1.6 tag so I had to install nodejs-legacy
to even be able to npm install
.
const transaction = await web3.eth.sendTransaction({ gas: gas, to: gnosisMultisigAddress, from: deployerAddress, data: gnosisFeeRegisterExitGame }); | ||
console.log(`Submitted transaction with hash for ${whatLog}: ${transaction.transactionHash}`); | ||
console.log(`Full log for ${whatLog}: ${transaction}`); | ||
await waitForReceipt(whatLog, transaction); | ||
}; | ||
|
||
const setVersion = async (whatLog, plasmaFramework, sha, gnosisMultisigAbi, gnosisMultisigAddress, deployerAddress) => { | ||
const setVersionCall = web3.eth.abi.encodeFunctionCall(plasmaFramework.abi.find(o => o.name === 'setVersion'), [`${pck.version}+${sha}`]); | ||
const gnosisSetVersion = web3.eth.abi.encodeFunctionCall(gnosisMultisigAbi, [plasmaFramework.address, 0, setVersionCall]); | ||
const gas = await web3.eth.estimateGas({ to: gnosisMultisigAddress, from: deployerAddress, data: gnosisSetVersion }); |
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.
do we not need *2
for this one?
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.
not sure, but at the moment even if it's reverted, there's no impact. the version I'll build after this PR will handle all these cases.
@@ -40,45 +40,55 @@ const waitForReceipt = async (whatLog, transaction) => { | |||
const setDepositVerifier = async (whatLog, ethVault, ethDepositVerifier, gnosisMultisigAbi, gnosisMultisigAddress, deployerAddress) => { | |||
const setDepositVerifierCall = web3.eth.abi.encodeFunctionCall(ethVault.abi.find(o => o.name === 'setDepositVerifier'), [ethDepositVerifier.address]); | |||
const gnosisSetDepositVerifier = web3.eth.abi.encodeFunctionCall(gnosisMultisigAbi, [ethVault.address, 0, setDepositVerifierCall]); | |||
const gas = await web3.eth.estimateGas({ to: gnosisMultisigAddress, from: deployerAddress, data: gnosisSetDepositVerifier }); | |||
const gas = await web3.eth.estimateGas({ to: gnosisMultisigAddress, from: deployerAddress, data: gnosisSetDepositVerifier }) * 2; |
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.
nitpick: the await web3.eth.estimateGas()
part is quite long, reading wise the *2
is not that obvious at my first glance. Might be slightly clearer with the following
const estimateGas = await web3.eth.estimateGas({ to: gnosisMultisigAddress, from: deployerAddress, data: gnosisSetDepositVerifier })
const transaction = await web3.eth.sendTransaction({ gas: estimateGas * 2, to: gnosisMultisigAddress, from: deployerAddress, data: gnosisSetDepositVerifier });
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.
makes sense!
also,
all of this is really not optimal at the moment, but it onblocks me on some other things. I will come back to this and make some adjustments - (like to be able to retry a certain transaction by maintaining some sort of state (like truffle migration does)). just FYI.
# chown -R nobody:nobody $CONFIG_DIR && chmod -R 777 $CONFIG_DIR | ||
} | ||
|
||
gencerts |
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 am not a script expert so I don't know what should be best practice, but shall we run commands after all function declaration so we don't mix reading command and function declaration together ??
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 is really just a copy/clone of what comes with vault. and it's purpose is to be able to standup and test the "migration integration" really works!
@@ -41,44 +41,54 @@ const setDepositVerifier = async (whatLog, ethVault, ethDepositVerifier, gnosisM | |||
const setDepositVerifierCall = web3.eth.abi.encodeFunctionCall(ethVault.abi.find(o => o.name === 'setDepositVerifier'), [ethDepositVerifier.address]); | |||
const gnosisSetDepositVerifier = web3.eth.abi.encodeFunctionCall(gnosisMultisigAbi, [ethVault.address, 0, setDepositVerifierCall]); | |||
const gas = await web3.eth.estimateGas({ to: gnosisMultisigAddress, from: deployerAddress, data: gnosisSetDepositVerifier }); | |||
const transaction = await web3.eth.sendTransaction({ gas: gas, to: gnosisMultisigAddress, from: deployerAddress, data: gnosisSetDepositVerifier }); | |||
console.log(`The amount of gas used for ${whatLog}: ${gas}`); |
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.
Can we log the correct value of gas sent here too ( *2 ) and for all the logs
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.
uuuuups!!!
|
||
# Don't exit until vault dies | ||
wait $VAULT_PID | ||
|
||
wait $VAULT_PID |
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.
a newline if you want
Bumping the Vault version to the latest release.