Merge pull request #151 from livepeer/yf/transcoder-self-bonded

Enforce transcoders to be self-bonded

Author: yondonfu

contracts/bonding/BondingManager.sol

@@ -170,12 +170,17 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
         // Fee share must be a valid percentage
         require(MathUtils.validPerc(_feeShare));
 
+        Delegator storage del = delegators[msg.sender];
+
+        // Must have a non-zero amount bonded to self
+        require(del.delegateAddress == msg.sender && del.bondedAmount > 0);
+
         Transcoder storage t = transcoders[msg.sender];
         t.pendingBlockRewardCut = _blockRewardCut;
         t.pendingFeeShare = _feeShare;
         t.pendingPricePerSegment = _pricePerSegment;
 
-        uint256 delegatedAmount = delegators[msg.sender].delegatedAmount;
+        uint256 delegatedAmount = del.delegatedAmount;
 
         // Check if transcoder is not already registered
         if (transcoderStatus(msg.sender) == TranscoderStatus.NotRegistered) {
@@ -197,29 +202,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
         TranscoderUpdate(msg.sender, _blockRewardCut, _feeShare, _pricePerSegment);
     }
 
-    /*
-     * @dev Remove the sender as a transcoder
-     */
-    function resignAsTranscoder()
-        external
-        whenSystemNotPaused
-        currentRoundInitialized
-    {
-        // Sender must be registered transcoder
-        require(transcoderStatus(msg.sender) == TranscoderStatus.Registered);
-
-        uint256 currentRound = roundsManager().currentRound();
-        if (activeTranscoderSet[currentRound].isActive[msg.sender]) {
-            // If transcoder is active for the round set it as inactive
-            activeTranscoderSet[currentRound].isActive[msg.sender] = false;
-        }
-
-        // Remove transcoder from pools
-        transcoderPool.remove(msg.sender);
-
-        TranscoderResigned(msg.sender);
-    }
-
     /**
      * @dev Delegate stake towards a specific address.
      * @param _amount The amount of LPT to stake.
@@ -316,12 +298,18 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
             transcoderPool.updateKey(del.delegateAddress, transcoderPool.getKey(del.delegateAddress).sub(del.bondedAmount), address(0), address(0));
         }
 
-        Unbond(del.delegateAddress, msg.sender);
-
         // Delegator no longer bonded to anyone
         del.delegateAddress = address(0);
         // Unbonding delegator does not have a start round
         del.startRound = 0;
+
+        // If caller is a registered transcoder, resign
+        // In the future, with partial unbonding there would be a check for 0 bonded stake as well
+        if (transcoderStatus(msg.sender) == TranscoderStatus.Registered) {
+            resignTranscoder(msg.sender);
+        }
+
+        Unbond(del.delegateAddress, msg.sender);
     }
 
     /**
@@ -804,6 +792,22 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
         return activeTranscoderSet[_round].isActive[_transcoder];
     }
 
+    /*
+     * @dev Remove transcoder
+     */
+    function resignTranscoder(address _transcoder) internal {
+        uint256 currentRound = roundsManager().currentRound();
+        if (activeTranscoderSet[currentRound].isActive[_transcoder]) {
+            // If transcoder is active for the round set it as inactive
+            activeTranscoderSet[currentRound].isActive[_transcoder] = false;
+        }
+
+        // Remove transcoder from pools
+        transcoderPool.remove(_transcoder);
+
+        TranscoderResigned(_transcoder);
+    }
+
     /*
      * @dev Update a transcoder with rewards
      * @param _transcoder Address of transcoder

contracts/jobs/JobsManager.sol

@@ -391,7 +391,6 @@ contract JobsManager is ManagerProxyTarget, IVerifiable, IJobsManager {
         // Sender must be elected transcoder
         require(job.transcoderAddress == msg.sender);
 
-        uint256 blockNum = roundsManager().blockNum();
         uint256 challengeBlock = claim.claimBlock + 1;
         // Segment must be eligible for verification
         // roundsManager().blockHash() ensures that the challenge block is within the last 256 blocks from the current block

contracts/test/GenericMock.sol

@@ -25,7 +25,7 @@ contract GenericMock {
      * @param _data Transaction data to be used to call the target contract
      */
     function execute(address _target, bytes _data) external returns (bool) {
-        // solium-disable security/no-low-level-calls
+        // solium-disable-next-line security/no-low-level-calls
         return _target.call(_data);
     }
 

test/unit/BondingManager.js

@@ -57,7 +57,18 @@ contract("BondingManager", accounts => {
             await expectThrow(bondingManager.transcoder(blockRewardCut, feeShare, pricePerSegment), {from: tAddr})
         })
 
-        it("should fail with zero delegated amount", async () => {
+        it("should fail if transcoder is not bonded to self", async () => {
+            await fixture.token.setApproved(true)
+            await bondingManager.bond(2000, tAddr, {from: accounts[2]})
+
+            // Fails because transcoder has non zero delegated stake but is not bonded to self
+            await expectThrow(bondingManager.transcoder(blockRewardCut, feeShare, pricePerSegment), {from: tAddr})
+        })
+
+        it("should fail if transcoder does not have a non-zero amount bonded to self", async () => {
+            await bondingManager.bond(0, tAddr, {from: tAddr})
+
+            // Fails because transcoder is delegated to self but has zero bonded stake
             await expectThrow(bondingManager.transcoder(blockRewardCut, feeShare, pricePerSegment), {from: tAddr})
         })
 
@@ -105,38 +116,6 @@ contract("BondingManager", accounts => {
         })
     })
 
-    describe("resignAsTranscoder", () => {
-        const tAddr = accounts[1]
-
-        beforeEach(async () => {
-            const blockRewardCut = 10 * PERC_MULTIPLIER
-            const feeShare = 5 * PERC_MULTIPLIER
-            const pricePerSegment = 100
-
-            await bondingManager.setParameters(UNBONDING_PERIOD, NUM_TRANSCODERS, NUM_ACTIVE_TRANSCODERS)
-
-            await fixture.roundsManager.setCurrentRoundInitialized(true)
-            await fixture.token.setApproved(true)
-            await bondingManager.bond(2000, tAddr, {from: tAddr})
-            await bondingManager.transcoder(blockRewardCut, feeShare, pricePerSegment, {from: tAddr})
-        })
-
-        it("should throw if transcoder is not registered", async () => {
-            await expectThrow(bondingManager.resignAsTranscoder({from: accounts[2]}))
-        })
-
-        it("should remove the transcoder from the transcoder pools", async () => {
-            await bondingManager.resignAsTranscoder({from: tAddr})
-            assert.equal(await bondingManager.transcoderStatus(tAddr), 0, "transcoder not removed from pool")
-        })
-
-        it("should set a transcoder as not registered", async () => {
-            await bondingManager.resignAsTranscoder({from: tAddr})
-            const transcoderStatus = await bondingManager.transcoderStatus(tAddr)
-            assert.equal(transcoderStatus, 0, "transcoder is not not registered")
-        })
-    })
-
     describe("bond", () => {
         const tAddr0 = accounts[1]
         const tAddr1 = accounts[2]
@@ -599,10 +578,10 @@ contract("BondingManager", accounts => {
             // Delegator bonds to transcoder
             await bondingManager.bond(2000, tAddr, {from: dAddr})
 
-            // Set active transcoders
-            await fixture.roundsManager.initializeRound()
             // Set current round so delegator is bonded
             await fixture.roundsManager.setCurrentRound(currentRound)
+            // Set active transcoders
+            await fixture.roundsManager.initializeRound()
         })
 
         it("should set withdraw round to current block + unbonding period", async () => {
@@ -642,6 +621,20 @@ contract("BondingManager", accounts => {
 
             assert.equal(startDelegatedAmount.sub(endDelegatedAmount), 2000, "delegate's delegated amount did not decrease by bonded amount")
         })
+
+        it("should resign transcoder if caller is a registered transcoder", async () => {
+            await bondingManager.unbond({from: tAddr})
+
+            assert.equal(await bondingManager.transcoderStatus(tAddr), 0, "transcoder not removed from pool")
+        })
+
+        it("should set transcoder as inactive for the current round if caller is a registered transcoder", async () => {
+            assert.isOk(await bondingManager.isActiveTranscoder(tAddr, currentRound), "transcoder should be active")
+
+            await bondingManager.unbond({from: tAddr})
+
+            assert.isNotOk(await bondingManager.isActiveTranscoder(tAddr, currentRound), "transcoder should be inactive")
+        })
     })
 
     describe("withdrawStake", () => {