Feat/eng 382 unbond function #15
Conversation
|
Code coverage decreased because I added an untested |
Pull Request Test Coverage Report for Build 102
💛 - Coveralls |
Pull Request Test Coverage Report for Build 104
💛 - Coveralls |
| // Only Bonded Delegators can call the function | ||
| require(delegatorStatus(msg.sender) == DelegatorStatus.Bonded); | ||
| // TODO: What if a Provider calls unbond() on himself ? | ||
| // Should he resign ? |
| require(delegatorStatus(msg.sender) == DelegatorStatus.Bonded); | ||
| // TODO: What if a Provider calls unbond() on himself ? | ||
| // Should he resign ? | ||
| // What about the tokens of the Delegators that bonded to him ? |
There was a problem hiding this comment.
Refunded - but who pays the gas cost for that? Him, I suppose.
There was a problem hiding this comment.
Yeah, but that could potentially be a huge cost in gas.
An alternative would be to let Delegators unbond() by themselves if they received the event that their Provider resigned.
There was a problem hiding this comment.
How does Livepeer handle this?
There was a problem hiding this comment.
They resign the Transcoder without refunding the delegators.
test/TestTransmuteDPOS.spec.js
Outdated
| const bondedAmount = (await tdpos.delegators(accounts[3]))[1].toNumber(); | ||
| const newAmount = totalBondedAmount - bondedAmount; | ||
| await tdpos.unbond({from: accounts[3]}); | ||
| assert.equal(newAmount, (await tdpos.providers([accounts[0]]))[5].toNumber()); |
There was a problem hiding this comment.
Can you annotate what the values associated with [0] and [5] are here?
|
|
||
| it('should emit the DelegateUnbonded event', async () => { | ||
| const delegator = await tdpos.delegators.call(accounts[5]); | ||
| assert.equal(accounts[0], delegator[0]); // [0] is delegateAddress; |
There was a problem hiding this comment.
Can we move these annotations to the first place that they are called in the test?
There was a problem hiding this comment.
What do you mean ? Isn't it the first place they are called in this test ?
There was a problem hiding this comment.
A better approach would be to use variables:
let delegateAddress = delegator[0]
let amountBonded = delegator[1]
test/TestTransmuteDPOS.spec.js
Outdated
| await approveBondProvider(22, 10, 1, 25, 1, accounts[0]); | ||
| }); | ||
|
|
||
| it('should return Unbonded if address in not a Delegator', async() => { |
test/TestTransmuteDPOS.spec.js
Outdated
| assert.equal(DELEGATOR_UNBONDED, await tdpos.delegatorStatus(accounts[3])); | ||
| }); | ||
|
|
||
| it('should return Bonded if delegator has called bond()', async() => { |
There was a problem hiding this comment.
You capitalized 'Delegator' in all other tests' descriptions, let's keep that consistent.
https://transmute.atlassian.net/browse/ENG-382