APEX-202 we should allow submitSignedBatch resubmit by batachers (#40)

contracts/Bridge.sol

@@ -140,13 +140,6 @@ contract Bridge is IBridge, Initializable, OwnableUpgradeable, UUPSUpgradeable {
  // Will determine if enough transactions are confirmed, or the timeout between two batches is exceeded.
  // It will also check if the given validator already submitted a signed batch and return the response accordingly.
  function shouldCreateBatch(string calldata _destinationChain) public view override returns (bool batch) {
- if (
- claims.isBatchCreated(_destinationChain) ||
- signedBatches.isBatchAlreadySubmittedBy(_destinationChain, msg.sender)
- ) {
- return false;
- }
-
  return claims.shouldCreateBatch(_destinationChain);
  }
 

contracts/Claims.sol

@@ -89,7 +89,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  continue;
  }
 
- _submitClaimsBRC(_claims, i, _caller);
+ _submitClaimsBRC(_claim, _caller);
  }
 
  uint256 batchExecutedClaimsLength = _claims.batchExecutedClaims.length;
@@ -107,7 +107,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  continue;
  }
 
- _submitClaimsBEC(_claims, i, _caller);
+ _submitClaimsBEC(_claim, _caller);
  }
 
  uint256 batchExecutionFailedClaimsLength = _claims.batchExecutionFailedClaims.length;
@@ -125,7 +125,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  continue;
  }
 
- _submitClaimsBEFC(_claims, i, _caller);
+ _submitClaimsBEFC(_claim, _caller);
  }
 
  uint256 refundRequestClaimsLength = _claims.refundRequestClaims.length;
@@ -143,7 +143,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  continue;
  }
 
- _submitClaimsRRC(_claims, i, _caller);
+ _submitClaimsRRC(_claim, _caller);
  }
 
  uint256 refundExecutedClaimsLength = _claims.refundExecutedClaims.length;
@@ -161,12 +161,11 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  continue;
  }
 
- _submitClaimsREC(_claims, i, _caller);
+ _submitClaimsREC(_claim, _caller);
  }
  }
 
- function _submitClaimsBRC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
- BridgingRequestClaim calldata _claim = _claims.bridgingRequestClaims[index];
+ function _submitClaimsBRC(BridgingRequestClaim calldata _claim, address _caller) internal {
  bytes32 claimHash = keccak256(abi.encode(_claim));
  uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);
 
@@ -192,8 +191,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  }
  }
 
- function _submitClaimsBEC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
- BatchExecutedClaim calldata _claim = _claims.batchExecutedClaims[index];
+ function _submitClaimsBEC(BatchExecutedClaim calldata _claim, address _caller) internal {
  bytes32 claimHash = keccak256(abi.encode(_claim));
  uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);
  string calldata chainID = _claim.chainID;
@@ -219,8 +217,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  }
  }
 
- function _submitClaimsBEFC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
- BatchExecutionFailedClaim calldata _claim = _claims.batchExecutionFailedClaims[index];
+ function _submitClaimsBEFC(BatchExecutionFailedClaim calldata _claim, address _caller) internal {
  bytes32 claimHash = keccak256(abi.encode(_claim));
  uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);
  string calldata chainID = _claim.chainID;
@@ -234,8 +231,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  }
  }
 
- function _submitClaimsRRC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
- RefundRequestClaim calldata _claim = _claims.refundRequestClaims[index];
+ function _submitClaimsRRC(RefundRequestClaim calldata _claim, address _caller) internal {
  bytes32 claimHash = keccak256(abi.encode(_claim));
  uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);
 
@@ -244,8 +240,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  }
  }
 
- function _submitClaimsREC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
- RefundExecutedClaim calldata _claim = _claims.refundExecutedClaims[index];
+ function _submitClaimsREC(RefundExecutedClaim calldata _claim, address _caller) internal {
  bytes32 claimHash = keccak256(abi.encode(_claim));
  uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);
 
@@ -273,11 +268,12 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
  return claimsHelper.setVoted(_id, _voter, _hash);
  }
 
- function isBatchCreated(string calldata _destinationChain) public view returns (bool batch) {
- return claimsHelper.currentBatchBlock(_destinationChain) != int(-1);
- }
-
  function shouldCreateBatch(string calldata _destinationChain) public view returns (bool) {
+ // if batch is already created, return false
+ if (claimsHelper.currentBatchBlock(_destinationChain) != int(-1)) {
+ return false;
+ }
+
  uint256 cnt = getBatchingTxsCount(_destinationChain);
 
  return cnt >= maxNumberOfTransactions || (cnt > 0 && block.number >= nextTimeoutBlock[_destinationChain]);

contracts/ClaimsHelper.sol

@@ -49,10 +49,6 @@ contract ClaimsHelper is IBridgeStructs, Initializable, OwnableUpgradeable, UUPS
  return confirmedSignedBatches[_chainId][_batchId];
  }
 
- function updateCurrentBatchBlock(string calldata _chainId) external onlySignedBatches {
- currentBatchBlock[_chainId] = int256(block.number);
- }
-
  function resetCurrentBatchBlock(string calldata _chainId) external onlyClaims {
  currentBatchBlock[_chainId] = int256(-1);
  }
@@ -72,6 +68,7 @@ contract ClaimsHelper is IBridgeStructs, Initializable, OwnableUpgradeable, UUPS
  confirmedSignedBatches[destinationChainId][signedBatchID].firstTxNonceId = _signedBatch.firstTxNonceId;
  confirmedSignedBatches[destinationChainId][signedBatchID].lastTxNonceId = _signedBatch.lastTxNonceId;
  confirmedSignedBatches[destinationChainId][signedBatchID].usedUTXOs = _signedBatch.usedUTXOs;
+ currentBatchBlock[destinationChainId] = int256(block.number);
  }
 
  function setVoted(

contracts/SignedBatches.sol

@@ -20,6 +20,8 @@ contract SignedBatches is IBridgeStructs, Initializable, OwnableUpgradeable, UUP
  // BlockchanID -> hash -> multisigSignatures
  mapping(string => mapping(bytes32 => string[])) private feePayerMultisigSignatures;
 
+ mapping(string => mapping(bytes32 => mapping(address => bool))) private hasVoted; // for resubmit
+
  // BlockchainID -> ConfirmedBatch
  mapping(string => ConfirmedBatch) public lastConfirmedBatch;
 
@@ -47,63 +49,55 @@ contract SignedBatches is IBridgeStructs, Initializable, OwnableUpgradeable, UUP
 
  function submitSignedBatch(SignedBatch calldata _signedBatch, address _caller) external onlyBridge {
  string calldata _destinationChainId = _signedBatch.destinationChainId;
- string memory _batchIdStr = Strings.toString(_signedBatch.id);
-
- uint256 sbId = lastConfirmedBatch[_destinationChainId].id;
+ uint256 _sbId = lastConfirmedBatch[_destinationChainId].id + 1;
 
- if (_signedBatch.id != sbId + 1) {
- return; // do not revert! batcher can lag a little bit. revert WrongBatchNonce(_destinationChainId, _signedBatch.id);
+ if (_signedBatch.id != _sbId) {
+ return; // skip if this is not batch we are expecting
  }
 
- if (claimsHelper.hasVoted(_batchIdStr, _caller)) {
- return;
- }
+ bytes32 _sbHash = keccak256(
+ abi.encode(
+ SignedBatchWithoutSignatures(
+ _signedBatch.id,
+ _destinationChainId,
+ _signedBatch.rawTransaction,
+ _signedBatch.firstTxNonceId,
+ _signedBatch.lastTxNonceId,
+ _signedBatch.usedUTXOs
+ )
+ )
+ );
 
- if (claimsHelper.isClaimConfirmed(_destinationChainId, _batchIdStr)) {
+ // check if caller already voted for same hash
+ if (hasVoted[_destinationChainId][_sbHash][_caller]) {
  return;
  }
 
- _submitSignedBatch(_signedBatch, _batchIdStr);
- }
-
- function _submitSignedBatch(SignedBatch calldata _signedBatch, string memory _batchId) internal {
- SignedBatchWithoutSignatures memory _signedBatchWithoutSignatures = SignedBatchWithoutSignatures(
- _signedBatch.id,
- _signedBatch.destinationChainId,
- _signedBatch.rawTransaction,
- _signedBatch.firstTxNonceId,
- _signedBatch.lastTxNonceId,
- _signedBatch.usedUTXOs
- );
- bytes32 signedBatchHash = keccak256(abi.encode(_signedBatchWithoutSignatures));
+ uint256 _quorumCount = validators.getQuorumNumberOfValidators();
+ uint256 _numberOfVotes = multisigSignatures[_destinationChainId][_sbHash].length;
 
- multisigSignatures[_signedBatch.destinationChainId][signedBatchHash].push(_signedBatch.multisigSignature);
- feePayerMultisigSignatures[_signedBatch.destinationChainId][signedBatchHash].push(
- _signedBatch.feePayerMultisigSignature
- );
-
- uint256 votesCount = claimsHelper.setVoted(_batchId, msg.sender, signedBatchHash);
-
- if (votesCount >= validators.getQuorumNumberOfValidators()) {
- claimsHelper.setConfirmedSignedBatchData(_signedBatch);
+ // check if consensus is already reached for this batch
+ if (_numberOfVotes >= _quorumCount) {
+ return;
+ }
 
- claimsHelper.setClaimConfirmed(_signedBatch.destinationChainId, _batchId);
+ hasVoted[_destinationChainId][_sbHash][_caller] = true;
+ multisigSignatures[_destinationChainId][_sbHash].push(_signedBatch.multisigSignature);
+ feePayerMultisigSignatures[_destinationChainId][_sbHash].push(_signedBatch.feePayerMultisigSignature);
 
- lastConfirmedBatch[_signedBatch.destinationChainId] = ConfirmedBatch(
- lastConfirmedBatch[_signedBatch.destinationChainId].id + 1,
+ // check if quorum reached (+1 is last vote)
+ if (_numberOfVotes + 1 >= _quorumCount) {
+ lastConfirmedBatch[_destinationChainId] = ConfirmedBatch(
+ _sbId,
  _signedBatch.rawTransaction,
- multisigSignatures[_signedBatch.destinationChainId][signedBatchHash],
- feePayerMultisigSignatures[_signedBatch.destinationChainId][signedBatchHash]
+ multisigSignatures[_destinationChainId][_sbHash],
+ feePayerMultisigSignatures[_destinationChainId][_sbHash]
  );
 
- claimsHelper.updateCurrentBatchBlock(_signedBatch.destinationChainId);
+ claimsHelper.setConfirmedSignedBatchData(_signedBatch);
  }
  }
 
- function isBatchAlreadySubmittedBy(string calldata _destinationChain, address addr) public view returns (bool ok) {
- return claimsHelper.hasVoted(Strings.toString(lastConfirmedBatch[_destinationChain].id + 1), addr);
- }
-
  function getConfirmedBatch(string calldata _destinationChain) external view returns (ConfirmedBatch memory batch) {
  return lastConfirmedBatch[_destinationChain];
  }

test/Bridge.test.ts

@@ -2192,6 +2192,14 @@ describe("Bridge Contract", function () {
  await bridge.connect(validators[0]).submitSignedBatch(signedBatch);
  await bridge.connect(validators[1]).submitSignedBatch(signedBatch);
  await bridge.connect(validators[2]).submitSignedBatch(signedBatch);
+
+ await bridge.connect(validators[1]).submitSignedBatch(signedBatch); // resubmit
+ const confBatchNothing = await claimsHelper
+ .connect(validators[0])
+ .getConfirmedSignedBatchData(signedBatch.destinationChainId, signedBatch.id);
+ expect(confBatchNothing.firstTxNonceId + confBatchNothing.lastTxNonceId).to.equal(0);
+
+ // consensus
  await bridge.connect(validators[3]).submitSignedBatch(signedBatch);
 
  const confBatch = await claimsHelper