Skip to content
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.

Sign up for GitHub

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

CLI relock fix #2464

Merged
merged 3 commits into from
Jan 21, 2020
Merged

CLI relock fix #2464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd8b620
added check to make sure that locked values is not zero
mrsmkl Jan 17, 2020
d7004fe
fixing flakiness
mrsmkl Jan 17, 2020
707b76f
Merge branch 'master' into relock-fix-2455
mrsmkl Jan 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/cli/src/commands/lockedgold/lock.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ testWithGanache('lockedgold:lock cmd', (web3: Web3) => {
await Lock.run(['--from', account, '--value', '100'])
await Unlock.run(['--from', account, '--value', '50'])
await Lock.run(['--from', account, '--value', '75'])
await Unlock.run(['--from', account, '--value', '50'])
await Lock.run(['--from', account, '--value', '50'])
const pendingWithdrawalsTotalValue = await lockedGold.getPendingWithdrawalsTotalValue(account)
expect(pendingWithdrawalsTotalValue.toFixed()).toBe('0')
})
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/commands/lockedgold/lock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ export default class Lock extends BaseCommand {
for (const txo of txos) {
await displaySendTx('relock', txo, { from: address })
}
const tx = lockedGold.lock()
await displaySendTx('lock', tx, { value: lockValue.toFixed() })
if (lockValue.gt(new BigNumber(0))) {
const tx = lockedGold.lock()
await displaySendTx('lock', tx, { value: lockValue.toFixed() })
}
}
}
19 changes: 14 additions & 5 deletions packages/protocol/test/governance/governance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ interface Transaction {
// hard coded in ganache
const EPOCH = 100

async function mineToNextEpoch(web3) {
const bn = web3.eth.getBlockNumber()
if (bn % EPOCH < 50) {
await mineBlocks(EPOCH + 20, web3)
} else {
await mineBlocks(EPOCH - 20, web3)
}
Comment on lines +80 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the rationale behind the +20 / -20?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well +20 is probably not useful, but -20 makes sure that the epoch won't change twice. Perhaps still need different handling for the case bn % EPOCH == 0.

}

// TODO(asa): Test dequeueProposalsIfReady
// TODO(asa): Dequeue explicitly to make the gas cost of operations more clear
contract('Governance', (accounts: string[]) => {
Expand Down Expand Up @@ -2083,7 +2092,7 @@ contract('Governance', (accounts: string[]) => {

describe('when hotfix is passing', () => {
beforeEach(async () => {
await mineBlocks(EPOCH, web3)
await mineToNextEpoch(web3)
await governance.whitelistHotfix(hotfixHashStr, { from: accounts[2] })
})

Expand Down Expand Up @@ -2116,7 +2125,7 @@ contract('Governance', (accounts: string[]) => {

it('should succeed for epoch != preparedEpoch', async () => {
await governance.prepareHotfix(hotfixHashStr)
await mineBlocks(EPOCH, web3)
await mineToNextEpoch(web3)
await governance.prepareHotfix(hotfixHashStr)
})
})
Expand All @@ -2138,7 +2147,7 @@ contract('Governance', (accounts: string[]) => {
})

it('should revert when hotfix not prepared for current epoch', async () => {
await mineBlocks(EPOCH, web3)
await mineToNextEpoch(web3)
await governance.approveHotfix(hotfixHashStr, { from: approver })
await assertRevert(executeHotfixTx())
})
Expand All @@ -2149,14 +2158,14 @@ contract('Governance', (accounts: string[]) => {
await accountsInstance.createAccount({ from: accounts[2] })
await governance.whitelistHotfix(hotfixHashStr, { from: accounts[2] })
await governance.prepareHotfix(hotfixHashStr, { from: accounts[2] })
await mineBlocks(EPOCH, web3)
await mineToNextEpoch(web3)
await assertRevert(executeHotfixTx())
})

describe('when hotfix is approved and prepared for current epoch', () => {
beforeEach(async () => {
await governance.approveHotfix(hotfixHashStr, { from: approver })
await mineBlocks(EPOCH, web3)
await mineToNextEpoch(web3)
await governance.addValidator(accounts[2])
await accountsInstance.createAccount({ from: accounts[2] })
await governance.whitelistHotfix(hotfixHashStr, { from: accounts[2] })
Expand Down