refactor(erc777): <- replace try/catch with low-level call

Contrary to try/catch, the low-level call to a non-contract address
does not revert, thus allowing to write user data in the transaction.

contracts/pToken.sol

@@ -15,6 +15,8 @@ contract PToken is
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
     bytes4 public ORIGIN_CHAIN_ID;
 
+    event ReceiveUserDataFailed();
+
     event Redeem(
         address indexed redeemer,
         uint256 value,
@@ -78,13 +80,15 @@ contract PToken is
         );
         _mint(recipient, value, userData, operatorData);
         if (userData.length > 0) {
-            try IPReceiver(recipient).receiveUserData(userData) {} catch {
-                // The recipient may be not implementing the IPReceiver interface,
-                // or the receiveUserData() hook may have a bug inside that would
-                // revert the entire transaction, thus blocking the minting process.
-                // Swallowing the error here permits pNetwork to fulfill its role
-                // of relaying messages/tokens across chains.
-            }
+            // pNetwork aims to deliver cross chain messages successfully regardless of what the user may do with them.
+            // We do not want this mint transaction reverting if their receiveUserData function reverts,
+            // and thus we swallow any such errors, emitting a HookFailed event instead.
+            // The low-level call is used because in the solidity version this contract was written in,
+            // a try/catch block fails to catch the revert caused if the receiver is not in fact a contract.
+            // This way, a user also has the option include userData even when minting to an externally owned account.
+            bytes memory data = abi.encodeWithSelector(IPReceiver.receiveUserData.selector, userData);
+            (bool success, ) = recipient.call(data);
+            if (!success) emit ReceiveUserDataFailed();
         }
         return true;
     }

contracts/pTokenNoGSN.sol

@@ -19,6 +19,8 @@ contract PTokenNoGSN is
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
     bytes4 public ORIGIN_CHAIN_ID;
 
+    event ReceiveUserDataFailed();
+
     event Redeem(
         address indexed redeemer,
         uint256 value,
@@ -81,13 +83,15 @@ contract PTokenNoGSN is
         );
         _mint(recipient, value, userData, operatorData);
         if (userData.length > 0) {
-            try IPReceiver(recipient).receiveUserData(userData) {} catch {
-                // The recipient may be not implementing the IPReceiver interface,
-                // or the receiveUserData() hook may have a bug inside that would
-                // revert the entire transaction, thus blocking the minting process.
-                // Swallowing the error here permits pNetwork to fulfill its role
-                // of relaying messages/tokens across chains.
-            }
+            // pNetwork aims to deliver cross chain messages successfully regardless of what the user may do with them.
+            // We do not want this mint transaction reverting if their receiveUserData function reverts,
+            // and thus we swallow any such errors, emitting a HookFailed event instead.
+            // The low-level call is used because in the solidity version this contract was written in,
+            // a try/catch block fails to catch the revert caused if the receiver is not in fact a contract.
+            // This way, a user also has the option include userData even when minting to an externally owned account.
+            bytes memory data = abi.encodeWithSelector(IPReceiver.receiveUserData.selector, userData);
+            (bool success, ) = recipient.call(data);
+            if (!success) emit ReceiveUserDataFailed();
         }
         return true;
     }

contracts/test-contracts/PReceiver.sol

@@ -9,3 +9,12 @@ contract PReceiver is IPReceiver {
         emit UserData(userData);
     }
 }
+
+contract PReceiverReverting is IPReceiver {
+    function receiveUserData(bytes calldata) external override {
+        require(false, "Revert!");
+    }
+}
+
+contract NotImplementing {
+}

test/05-ptoken.test.js

@@ -145,21 +145,24 @@ USE_GSN.map(_useGSN =>
         }
       })
 
-      it('`mint()` w/ data should revert if receiver is not a contract', async () => {
+      it('`mint()` w/ data should mint tokens to a non-contract address & emit correct events', async () => {
         const data = '0xdead'
+        const expectedNumEvents = 2
         const operatorData = '0xb33f'
         const recipient = OTHERS[0].address
-        try {
-          const tx = await CONTRACT['mint(address,uint256,bytes,bytes)'](recipient, AMOUNT, data, operatorData)
-          await tx.wait()
-          assert.fail('Should not succeed!')
-        } catch (_err) {
-          const expectedErr = 'Transaction reverted: function call to a non-contract account'
-          assert(_err.message.includes(expectedErr))
-        }
+        const recipientBalanceBefore = await getTokenBalance(recipient, CONTRACT)
+        const tx =
+          await CONTRACT['mint(address,uint256,bytes,bytes)'](recipient, AMOUNT, data, operatorData)
+        const { events } = await tx.wait()
+        const recipientBalanceAfter = await getTokenBalance(recipient, CONTRACT)
+        assert(recipientBalanceBefore.eq(BigNumber.from(0)))
+        assert(recipientBalanceAfter.eq(BigNumber.from(AMOUNT)))
+        assert.strictEqual(events.length, expectedNumEvents)
+        assertTransferEvent(events, ZERO_ADDRESS, recipient, AMOUNT)
+        assertMintEvent(events, recipient, OWNER.address, AMOUNT, data, operatorData)
       })
 
-      it('`mint()` w/ data should mint tokens & emit correct events', async () => {
+      it('`mint()` w/ data should mint & call receiveUserData & emit correct events', async () => {
         const data = '0xdead'
         const expectedNumEvents = 3
         const operatorData = '0xb33f'
@@ -178,9 +181,56 @@ USE_GSN.map(_useGSN =>
         assertTransferEvent(events, ZERO_ADDRESS, recipientContract.address, AMOUNT)
         assertMintEvent(events, recipientContract.address, OWNER.address, AMOUNT, data, operatorData)
         const userDataEvent = recipientContract.interface.parseLog(events.at(-1))
+        assert.strictEqual(userDataEvent.name, 'UserData')
         assert.strictEqual(userDataEvent.args.data, data)
       })
 
+      // eslint-disable-next-line max-len
+      it('`mint()` w/ `userData` should mint tokens, emit correct events & not revert despite `receiveUserData` hook being not implemented',
+        async () => {
+          const data = '0xdead'
+          const expectedNumEvents = 3
+          const operatorData = '0xb33f'
+          const recipientContract = await ethers
+            .getContractFactory('contracts/test-contracts/PReceiver.sol:NotImplementing')
+            .then(_factory => upgrades.deployProxy(_factory))
+
+          const recipientBalanceBefore = await getTokenBalance(recipientContract.address, CONTRACT)
+          const tx =
+            await CONTRACT['mint(address,uint256,bytes,bytes)'](recipientContract.address, AMOUNT, data, operatorData)
+          const { events } = await tx.wait()
+          const recipientBalanceAfter = await getTokenBalance(recipientContract.address, CONTRACT)
+          assert(recipientBalanceBefore.eq(BigNumber.from(0)))
+          assert(recipientBalanceAfter.eq(BigNumber.from(AMOUNT)))
+          assert.strictEqual(events.length, expectedNumEvents)
+          assertTransferEvent(events, ZERO_ADDRESS, recipientContract.address, AMOUNT)
+          assertMintEvent(events, recipientContract.address, OWNER.address, AMOUNT, data, operatorData)
+          assert.strictEqual(events.at(-1).event, 'ReceiveUserDataFailed')
+        })
+
+      // eslint-disable-next-line max-len
+      it('`mint()` w/ `userData` should mint tokens, emit correct events & not revert despite `receiveUserData` hook reverting',
+        async () => {
+          const data = '0xdead'
+          const expectedNumEvents = 3
+          const operatorData = '0xb33f'
+          const recipientContract = await ethers
+            .getContractFactory('contracts/test-contracts/PReceiver.sol:PReceiverReverting')
+            .then(_factory => upgrades.deployProxy(_factory))
+
+          const recipientBalanceBefore = await getTokenBalance(recipientContract.address, CONTRACT)
+          const tx =
+            await CONTRACT['mint(address,uint256,bytes,bytes)'](recipientContract.address, AMOUNT, data, operatorData)
+          const { events } = await tx.wait()
+          const recipientBalanceAfter = await getTokenBalance(recipientContract.address, CONTRACT)
+          assert(recipientBalanceBefore.eq(BigNumber.from(0)))
+          assert(recipientBalanceAfter.eq(BigNumber.from(AMOUNT)))
+          assert.strictEqual(events.length, expectedNumEvents)
+          assertTransferEvent(events, ZERO_ADDRESS, recipientContract.address, AMOUNT)
+          assertMintEvent(events, recipientContract.address, OWNER.address, AMOUNT, data, operatorData)
+          assert.strictEqual(events.at(-1).event, 'ReceiveUserDataFailed')
+        })
+
       it('Should revert when minting tokens with the contract address as the recipient', async () => {
         const recipient = CONTRACT.address
         try {