fix(precompiles): return data for revert (#224)

* fix(vm/keeper): add return data of ApplyMessageWithConfig for ErrExecutionReverted

* fix(precompile): modify return data of distribution precompile for revert error

* test: remove redundant test case

* chore(precompiles/staking): modify description for integration test case

* chore: fix lint

* fix(precompiles): modify return data of precompiles for revert error

* fix: broken test cases after modifying precompile err to revert err

* refactor:(precompiles) convert error that precompile.Run returns to ErrExecutionReverted

* wip: test(precompiles/staking): fix test cases

* chore: compile latest test contracts

* fix(precompiles/staking): check revert reason or integration test cases

* test(precompiles/staking): fix unit test

* test(precompiles/distribution): improve integration test

* test(precompiles/erc20): improve integration test

* test(precompiles/ics20): improve integration test

* test(precompiles/slashing): add slashing integration test for proof of audit issue fix

* chore: fix lint

* chore: fix lint

Author: cloudgray

contracts/solidity/precompiles/gov/IGov.sol

@@ -21,8 +21,8 @@ enum VoteOption {
     Abstain,
     // No defines a no vote option.
     No,
-    // NoWithWeto defines a no with veto vote option.
-    NoWithWeto
+    // NoWithVeto defines a no with veto vote option.
+    NoWithVeto
 }
 /// @dev WeightedVote represents a vote on a governance proposal
 struct WeightedVote {

contracts/solidity/precompiles/slashing/testdata/SlashingCaller.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity >=0.8.17;
+
+import "../ISlashing.sol" as slashing;
+
+contract SlashingCaller {
+    event TestResult(string message, bool success);
+
+    function testUnjail(address validatorAddr) public returns (bool success) {
+        return slashing.SLASHING_CONTRACT.unjail(validatorAddr);
+    }
+}

contracts/solidity/precompiles/staking/testdata/DelegationManager.sol

@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity >=0.8.17;
+
+contract DelegationManager {
+
+    /// The delegation mapping is used to associate the EOA address that 
+    /// actually made the delegate request with its corresponding delegation information. 
+    mapping(address => mapping(string => uint256)) public delegations;
+     
+    /// The unbonding queue is used to store the unbonding operations that are in progress.
+    mapping(address => UnbondingDelegation[]) public unbondingDelegations;
+
+    /// The unbonding entry struct represents an unbonding operation that is in progress. 
+    /// It contains information about the validator and the amount of tokens that are being unbonded.
+    struct UnbondingDelegation {
+        /// @dev The validator address is the address of the validator that is being unbonded.
+        string validator;
+        /// @dev The amount of tokens that are being unbonded.
+        uint256 amount;
+        /// @dev The creation height is the height at which the unbonding operation was created.
+        uint256 creationHeight;
+        /// @dev The completion time is the time at which the unbonding operation will complete.
+        int64 completionTime;
+    }
+
+    function _increaseAmount(address _delegator, string memory _validator, uint256 _amount) internal {
+        delegations[_delegator][_validator] += _amount;
+    }
+
+    function _decreaseAmount(address _delegator, string memory _validator, uint256 _amount) internal {
+        require(delegations[_delegator][_validator] >= _amount, "Insufficient delegation amount");
+        delegations[_delegator][_validator] -= _amount;
+    }
+
+    function _undelegate(string memory _validatorAddr, uint256 _amount, int64 completionTime) internal {
+        unbondingDelegations[msg.sender].push(UnbondingDelegation({
+            validator: _validatorAddr,
+            amount: _amount,
+            creationHeight: block.number,
+            completionTime: completionTime
+        }));
+    }
+
+    /// @dev This function is used to dequeue unbonding entries that have expired.
+    ///
+    /// @notice StakingCaller acts as the delegator and manages delegation/unbonding state per EoA.
+    /// Reflecting x/staking unbondingDelegations changes in real-time would require event listening.
+    /// To simplify unbonding entry processing, this function is called during delegate/undelegate calls.
+    /// Although updating unbondingDelegations state isn't tested in the staking precompile integration tests,
+    /// it is included for the completeness of the contract.
+    function _dequeueUnbondingDelegation() internal {
+        for (uint256 i = 0; i < unbondingDelegations[msg.sender].length; i++) {
+            UnbondingDelegation storage entry = unbondingDelegations[msg.sender][i];
+            if (uint256(int256(entry.completionTime)) <= block.timestamp) {
+                delete unbondingDelegations[msg.sender][i];
+                delegations[msg.sender][entry.validator] -= entry.amount;
+            }
+        }
+    }
+
+    /// @dev This function is used to cancel unbonding entries that have been cancelled.
+    /// @param _creationHeight The creation height of the unbonding entry to cancel.
+    /// @param _amount The amount to cancel.
+    function _cancelUnbonding(uint256 _creationHeight, uint256 _amount) internal {
+        UnbondingDelegation[] storage entries = unbondingDelegations[msg.sender];
+
+        for (uint256 i = 0; i < entries.length; i++) {
+            UnbondingDelegation storage entry = entries[i];
+
+            if (entry.creationHeight != _creationHeight) { continue; }
+
+            require(entry.amount >= _amount, "amount exceeds unbonding entry amount");
+            entry.amount -= _amount;
+
+            // If the amount is now 0, remove the entry
+            if (entry.amount == 0) { delete entries[i]; }
+
+            // Only cancel one entry per call
+            break; 
+        }
+    }
+
+    function _checkDelegation(string memory _validatorAddr, uint256 _delegateAmount) internal view {
+        require(
+            delegations[msg.sender][_validatorAddr] >= _delegateAmount, 
+            "Delegation does not exist or insufficient delegation amount"
+        );
+    }
+
+    function _checkUnbondingDelegation(address _delegatorAddr, string memory _validatorAddr) internal view {
+        bool found;
+        for (uint256 i = 0; i < unbondingDelegations[_delegatorAddr].length; i++) {
+            UnbondingDelegation storage entry = unbondingDelegations[_delegatorAddr][i];
+            if (
+                _equalStrings(entry.validator, _validatorAddr) && 
+                uint256(int256(entry.completionTime)) > block.timestamp
+            ) {
+                found = true;
+                break;
+            }
+        }
+        require(found == true, "Unbonding delegation does not exist");
+    }
+
+    function _equalStrings(string memory a, string memory b) internal pure returns (bool) {
+        return keccak256(bytes(a)) == keccak256(bytes(b));
+    }
+}

contracts/solidity/precompiles/staking/testdata/StakingCaller.sol

@@ -2,36 +2,17 @@
 pragma solidity >=0.8.17;
 
 import "../StakingI.sol" as staking;
+import "./DelegationManager.sol";
 
 /// @title StakingCaller
 /// @author Evmos Core Team
 /// @dev This contract is used to test external contract calls to the staking precompile.
-contract StakingCaller {
+contract StakingCaller is DelegationManager{
     /// counter is used to test the state persistence bug, when EVM and Cosmos state were both
     /// changed in the same function.
     uint256 public counter;
     string[] private delegateMethod = [staking.MSG_DELEGATE];
 
-    /// The delegation mapping is used to associate the EOA address that 
-    /// actually made the delegate request with its corresponding delegation information. 
-    mapping(address => mapping(string => uint256)) public delegation;
-
-    /// The unbonding entry struct represents an unbonding operation that is in progress. 
-    /// It contains information about the validator and the amount of tokens that are being unbonded.
-    struct UnbondingEntry {
-        /// @dev The validator address is the address of the validator that is being unbonded.
-        string validator;
-        /// @dev The amount of tokens that are being unbonded.
-        uint256 amount;
-        /// @dev The creation height is the height at which the unbonding operation was created.
-        uint256 creationHeight;
-        /// @dev The completion time is the time at which the unbonding operation will complete.
-        int64 completionTime;
-    }
-
-    /// The unbonding queue is used to store the unbonding operations that are in progress.
-    mapping(address => UnbondingEntry[]) public unbondingQueue;
-
     /// @dev This function calls the staking precompile's create validator method
     /// using the msg.sender as the validator's operator address.
     /// @param _descr The initial description
@@ -91,15 +72,14 @@ contract StakingCaller {
     function testDelegate(
         string memory _validatorAddr
     ) public payable {
-        _dequeueUnbondingEntry();
-
+        _dequeueUnbondingDelegation();
         bool success = staking.STAKING_CONTRACT.delegate(
             address(this),
             _validatorAddr,
             msg.value
         );
         require(success, "delegate failed");
-        delegation[msg.sender][_validatorAddr] += msg.value;
+        _increaseAmount(msg.sender, _validatorAddr, msg.value);
     }
 
     /// @dev This function calls the staking precompile's undelegate method.
@@ -109,20 +89,11 @@ contract StakingCaller {
         string memory _validatorAddr,
         uint256 _amount
     ) public {
-        _dequeueUnbondingEntry();
-
-        require(delegation[msg.sender][_validatorAddr] >= _amount, "Insufficient delegation");
-        
+        _checkDelegation(_validatorAddr, _amount);
+        _dequeueUnbondingDelegation();
         int64 completionTime = staking.STAKING_CONTRACT.undelegate(address(this), _validatorAddr, _amount);
         require(completionTime > 0, "Failed to undelegate");
-
-        uint256 creationHeight = block.number;
-        unbondingQueue[msg.sender].push(UnbondingEntry({
-            validator: _validatorAddr,
-            amount: _amount,
-            creationHeight: creationHeight,
-            completionTime: completionTime
-        }));
+        _undelegate(_validatorAddr, _amount, completionTime);
     }
 
     /// @dev This function calls the staking precompile's redelegate method.
@@ -133,16 +104,17 @@ contract StakingCaller {
         string memory _validatorSrcAddr,
         string memory _validatorDstAddr,
         uint256 _amount
-    ) public {        
+    ) public {
+        _checkDelegation(_validatorSrcAddr, _amount);
         int64 completionTime = staking.STAKING_CONTRACT.redelegate(
             address(this),
             _validatorSrcAddr,
             _validatorDstAddr,
             _amount
         );
         require(completionTime > 0, "Failed to redelegate");
-        delegation[msg.sender][_validatorSrcAddr] -= _amount;
-        delegation[msg.sender][_validatorDstAddr] += _amount;
+        _decreaseAmount(msg.sender, _validatorSrcAddr, _amount);
+        _increaseAmount(msg.sender, _validatorDstAddr, _amount);
     }
 
     /// @dev This function calls the staking precompile's cancel unbonding delegation method.
@@ -154,16 +126,15 @@ contract StakingCaller {
         uint256 _amount,
         uint256 _creationHeight
     ) public {
-        _dequeueUnbondingEntry();
-
+        _dequeueUnbondingDelegation();
+        _checkUnbondingDelegation(msg.sender, _validatorAddr);        
         bool success = staking.STAKING_CONTRACT.cancelUnbondingDelegation(
             address(this),
             _validatorAddr,
             _amount,
             _creationHeight
         );
         require(success, "Failed to cancel unbonding");
-
         _cancelUnbonding(_creationHeight, _amount);
     }
 
@@ -280,8 +251,7 @@ contract StakingCaller {
         uint256 _amount,
         string memory _calltype
     ) public {
-        _dequeueUnbondingEntry();
-
+        _dequeueUnbondingDelegation();
         address calledContractAddress = staking.STAKING_PRECOMPILE_ADDRESS;
         bytes memory payload = abi.encodeWithSignature(
             "undelegate(address,string,uint256)",
@@ -331,14 +301,7 @@ contract StakingCaller {
         } else {
             revert("invalid calltype");
         }
-
-        uint256 creationHeight = block.number;
-        unbondingQueue[msg.sender].push(UnbondingEntry({
-            validator: _validatorAddr,
-            amount: _amount,
-            creationHeight: creationHeight,
-            completionTime: completionTime
-        }));
+        _undelegate(_validatorAddr, _amount, completionTime);
     }
 
     /// @dev This function is used to test the behaviour when executing queries using special function calling opcodes,
@@ -452,15 +415,14 @@ contract StakingCaller {
     function testDelegateIncrementCounter(
         string memory _validatorAddr
     ) public payable {
-        _dequeueUnbondingEntry();
-
+        _dequeueUnbondingDelegation();
         bool success = staking.STAKING_CONTRACT.delegate(
             address(this),
             _validatorAddr,
             msg.value
         );
         require(success, "delegate failed");
-        delegation[msg.sender][_validatorAddr] += msg.value;
+        _increaseAmount(msg.sender, _validatorAddr, msg.value);
         counter += 1;
     }
 
@@ -469,15 +431,14 @@ contract StakingCaller {
     function testDelegateAndFailCustomLogic(
         string memory _validatorAddr
     ) public payable {
-        _dequeueUnbondingEntry();
-
+        _dequeueUnbondingDelegation();
         bool success = staking.STAKING_CONTRACT.delegate(
             address(this),
             _validatorAddr,
             msg.value
         );
         require(success, "delegate failed");
-        delegation[msg.sender][_validatorAddr] += msg.value;
+        _increaseAmount(msg.sender, _validatorAddr, msg.value);
 
         // This should fail since the balance is already spent in the previous call
         payable(msg.sender).transfer(msg.value);
@@ -496,8 +457,7 @@ contract StakingCaller {
         string memory _validatorAddr,
         uint256 _amount
     ) public payable {
-        _dequeueUnbondingEntry();
-
+        _dequeueUnbondingDelegation();
         (bool success, ) = _contract.call(
             abi.encodeWithSignature(
                 "transfer(address,uint256)",
@@ -506,53 +466,8 @@ contract StakingCaller {
             )
         );
         require(success, "transfer failed");
-
         success = staking.STAKING_CONTRACT.delegate(address(this), _validatorAddr, msg.value);
         require(success, "delegate failed");
-        delegation[msg.sender][_validatorAddr] += msg.value;
-    }
-
-    /// @dev This function is used to dequeue unbonding entries that have expired.
-    ///
-    /// @notice StakingCaller acts as the delegator and manages delegation/unbonding state per EoA.
-    /// Reflecting x/staking unbondingQueue changes in real-time would require event listening.
-    /// To simplify unbonding entry processing, this function is called during delegate/undelegate calls.
-    /// Although updating unbondingQueue state isn't tested in the staking precompile integration tests,
-    /// it is included for the completeness of the contract.
-    function _dequeueUnbondingEntry() private {
-        
-        for (uint256 i = 0; i < unbondingQueue[msg.sender].length; i++) {
-            UnbondingEntry storage entry = unbondingQueue[msg.sender][i];
-            if (uint256(int256(entry.completionTime)) <= block.timestamp) {
-                delete unbondingQueue[msg.sender][i];
-                delegation[msg.sender][entry.validator] -= entry.amount;
-            }
-        }
-    }
-
-    /// @dev This function is used to cancel unbonding entries that have been cancelled.
-    /// @param _creationHeight The creation height of the unbonding entry to cancel.
-    /// @param _amount The amount to cancel.
-    function _cancelUnbonding(uint256 _creationHeight, uint256 _amount) private {
-        UnbondingEntry[] storage entries = unbondingQueue[msg.sender];
-
-        for (uint256 i = 0; i < entries.length; i++) {
-            UnbondingEntry storage entry = entries[i];
-
-            if (entry.creationHeight != _creationHeight) {
-                continue;
-            }
-
-            require(entry.amount >= _amount, "amount exceeds unbonding entry amount");
-            entry.amount -= _amount;
-
-            // If the amount is now 0, remove the entry
-            if (entry.amount == 0) {
-                delete entries[i];
-            }
-
-            // Only cancel one entry per call
-            break; 
-        }
+        _increaseAmount(msg.sender, _validatorAddr, msg.value);
     }
 }

contracts/solidity/precompiles/testutil/contracts/StakingReverter.sol

@@ -31,6 +31,10 @@ contract StakingReverter {
         }
     }
 
+    /// @dev callPrecompileBeforeAndAfterRevert tests whether precompile calls that occur 
+    /// before and after an intentionally ignored revert correctly modify the state.
+    /// This method assumes that the StakingReverter.sol contract holds a native balance. 
+    /// Therefore, in order to call this method, the contract must be funded with a balance in advance.
     function callPrecompileBeforeAndAfterRevert(uint numTimes, string calldata validatorAddress) external {
         STAKING_CONTRACT.delegate(address(this), validatorAddress, 10);