removed approval requirement for cross chain transfers

Author: aalavandhan

contracts/satellite-chain/xc-ampleforth/XCAmple.sol

@@ -374,6 +374,7 @@ contract XCAmple is IERC20Upgradeable, OwnableUpgradeable {
 
     /**
      * @dev Mint xcAmples to a beneficiary.
+     *      Only callable by the token controller.
      *
      * @param who The address of the beneficiary.
      * @param xcAmpleAmount The amount of xcAmple tokens to be minted.
@@ -390,25 +391,15 @@ contract XCAmple is IERC20Upgradeable, OwnableUpgradeable {
     }
 
     /**
-     * @dev Destroys `xcAmpleAmount` tokens from `who`, deducting from the caller's
-     * allowance.
-     *
-     * This is only callable by the controller, because we only want to support burn for the
-     * interchain-transfer case. Otherwise, AMPL on base chain could become locked in the vault.
-     *
-     * Requirements:
-     *
-     * - the caller must have allowance for ``who``'s tokens of at least
-     * `xcAmpleAmount`.
+     * @dev Destroys `xcAmpleAmount` tokens from `who`.
+     *      Only callable by the token controller.
      *
      * @param who The address of the beneficiary.
      * @param xcAmpleAmount The amount of xcAmple tokens to be burned.
      */
     function burnFrom(address who, uint256 xcAmpleAmount) external onlyController {
         require(who != address(0), "XCAmple: burn address zero address");
 
-        _allowedXCAmples[who][msg.sender] = _allowedXCAmples[who][msg.sender].sub(xcAmpleAmount);
-
         uint256 gonValue = xcAmpleAmount.mul(_gonsPerAMPL);
         _gonBalances[who] = _gonBalances[who].sub(gonValue);
         _totalSupply = _totalSupply.sub(xcAmpleAmount);

test/integration/chain_bridge.js

@@ -259,9 +259,7 @@ async function execXCSend (
         .connect(fromAccount)
         .approve(baseChainAmplContracts.tokenVault.address, amount);
     } else {
-      await amplContractsMap[fromChain].xcAmple
-        .connect(fromAccount)
-        .approve(amplContractsMap[fromChain].xcAmpleController.address, amount);
+      // NO Approval required!
     }
   }
 
@@ -764,7 +762,7 @@ describe('Transfers scenarios', function () {
     });
 
     describe('user does not approve Controller on satellite chain', function () {
-      it('should revert', async function () {
+      it('should NOT revert', async function () {
         // Setup by placing amples on satellite chain
         await checkBalancesAndSupply(
           ['100000', '100000'],
@@ -804,7 +802,7 @@ describe('Transfers scenarios', function () {
             toAmplFixedPt('1'),
             false, // Use existing approval and don't auto-approve.
           ),
-        ).to.be.reverted;
+        ).not.to.be.reverted;
       });
     });
   });

test/unit/satellite-chain/xc-ampleforth/xc_ample_burn.js

@@ -36,9 +36,6 @@ describe('XCAmple:burnFrom:accessControl', function () {
     await xcAmple
       .connect(deployer)
       .mint(otherUser.getAddress(), unitTokenAmount);
-    await xcAmple
-      .connect(otherUser)
-      .approve(await deployer.getAddress(), unitTokenAmount);
   });
 
   it('should NOT be callable by other user', async function () {
@@ -74,37 +71,19 @@ describe('XCAmple:burnFrom', () => {
       const mintAmt = toUFrgDenomination('1000000');
       await xcAmple.connect(deployer).mint(otherUser.getAddress(), mintAmt);
       const burnAmt = (await xcAmple.balanceOf(otherUser.getAddress())).add(1);
-      await xcAmple
-        .connect(otherUser)
-        .approve(await deployer.getAddress(), burnAmt);
-
       await expect(
         xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt),
       ).to.be.reverted;
     });
   });
 
-  describe('when burn value > approved amount', () => {
-    it('should revert', async function () {
-      const mintAmt = toUFrgDenomination('1000000');
-      await xcAmple.connect(deployer).mint(otherUser.getAddress(), mintAmt);
-      await xcAmple
-        .connect(otherUser)
-        .approve(await deployer.getAddress(), mintAmt.sub(1));
-      await expect(
-        xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), mintAmt),
-      ).to.be.reverted;
-    });
-  });
-
   describe('when burn value = user balance', () => {
     const amt1 = toUFrgDenomination('1000000');
     const amt2 = toUFrgDenomination('12343588');
     const totalAmt = amt1.add(amt2);
     beforeEach(async function () {
       await xcAmple.connect(deployer).mint(deployer.getAddress(), amt2);
       await xcAmple.connect(deployer).mint(otherUser.getAddress(), amt1);
-      await xcAmple.connect(otherUser).approve(deployer.getAddress(), amt1);
     });
     it('should burn tokens from wallet', async function () {
       await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), amt1);
@@ -120,16 +99,6 @@ describe('XCAmple:burnFrom', () => {
       await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), amt1);
       expect(await xcAmple.totalSupply()).to.eq(amt2);
     });
-    it('should reduce the approved amount', async function () {
-      const allowanceBefore = await xcAmple.allowance(
-        otherUser.getAddress(),
-        deployer.getAddress(),
-      );
-      await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), amt1);
-      expect(
-        await xcAmple.allowance(otherUser.getAddress(), deployer.getAddress()),
-      ).to.eq(allowanceBefore - amt1);
-    });
     it('should log Transfer to zero address', async function () {
       await expect(
         xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), amt1),
@@ -150,7 +119,6 @@ describe('XCAmple:burnFrom', () => {
 
     beforeEach(async function () {
       await xcAmple.connect(deployer).mint(otherUser.getAddress(), mintAmt);
-      await xcAmple.connect(otherUser).approve(deployer.getAddress(), mintAmt);
     });
     it('should burn tokens from wallet', async function () {
       await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt);
@@ -163,16 +131,6 @@ describe('XCAmple:burnFrom', () => {
       await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt);
       expect(await xcAmple.totalSupply()).to.eq(remainingBal);
     });
-    it('should reduce the approved amount', async function () {
-      const allowanceBefore = await xcAmple.allowance(
-        otherUser.getAddress(),
-        deployer.getAddress(),
-      );
-      await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt);
-      expect(
-        await xcAmple.allowance(otherUser.getAddress(), deployer.getAddress()),
-      ).to.eq(allowanceBefore - burnAmt);
-    });
     it('should log Transfer to zero address', async function () {
       await expect(
         xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt),
@@ -193,9 +151,6 @@ describe('XCAmple:burnFrom', () => {
     beforeEach(async function () {
       await xcAmple.rebase(1, MAX_SUPPLY);
       await xcAmple.connect(deployer).mint(otherUser.getAddress(), MAX_SUPPLY);
-      await xcAmple
-        .connect(otherUser)
-        .approve(deployer.getAddress(), MAX_SUPPLY);
     });
 
     it('should burn tokens from wallet', async function () {
@@ -209,16 +164,6 @@ describe('XCAmple:burnFrom', () => {
       await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt);
       expect(await xcAmple.totalSupply()).to.eq(remainingBal);
     });
-    it('should reduce the approved amount', async function () {
-      const allowanceBefore = await xcAmple.allowance(
-        otherUser.getAddress(),
-        deployer.getAddress(),
-      );
-      await xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt);
-      expect(
-        await xcAmple.allowance(otherUser.getAddress(), deployer.getAddress()),
-      ).to.eq(allowanceBefore.sub(burnAmt));
-    });
     it('should log Transfer to zero address', async function () {
       await expect(
         xcAmple.connect(deployer).burnFrom(otherUser.getAddress(), burnAmt),