Skip to content

Commit

Permalink
Optimization memory to calldata (#34)
Browse files Browse the repository at this point in the history
* replaced memory with calldata where possible and removed some unused functions

* few more

* return instead of revert

* test fix

---------

Co-authored-by: Igor Crevar <[email protected]>
  • Loading branch information
MiroslavStefanovic and igorcrevar committed May 15, 2024
1 parent 09d9084 commit 90ddab7
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 43 deletions.
3 changes: 1 addition & 2 deletions contracts/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ contract Bridge is IBridge, Initializable, OwnableUpgradeable, UUPSUpgradeable {
// Batches
function submitSignedBatch(SignedBatch calldata _signedBatch) external override onlyValidator {
if (!shouldCreateBatch(_signedBatch.destinationChainId)) {
// it will revert also if chain is not registered
revert CanNotCreateBatchYet(_signedBatch.destinationChainId);
return;
}
if (
!validators.isSignatureValid(
Expand Down
29 changes: 14 additions & 15 deletions contracts/Claims.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad

function submitClaims(ValidatorClaims calldata _claims, address _caller) external onlyBridge {
for (uint i = 0; i < _claims.bridgingRequestClaims.length; i++) {
BridgingRequestClaim memory _claim = _claims.bridgingRequestClaims[i];
BridgingRequestClaim calldata _claim = _claims.bridgingRequestClaims[i];
if (!isChainRegistered[_claim.sourceChainID]) {
revert ChainIsNotRegistered(_claim.sourceChainID);
}
Expand All @@ -88,7 +88,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
_submitClaimsBRC(_claims, i, _caller);
}
for (uint i = 0; i < _claims.batchExecutedClaims.length; i++) {
BatchExecutedClaim memory _claim = _claims.batchExecutedClaims[i];
BatchExecutedClaim calldata _claim = _claims.batchExecutedClaims[i];
if (!isChainRegistered[_claim.chainID]) {
revert ChainIsNotRegistered(_claim.chainID);
}
Expand All @@ -104,7 +104,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
_submitClaimsBEC(_claims, i, _caller);
}
for (uint i = 0; i < _claims.batchExecutionFailedClaims.length; i++) {
BatchExecutionFailedClaim memory _claim = _claims.batchExecutionFailedClaims[i];
BatchExecutionFailedClaim calldata _claim = _claims.batchExecutionFailedClaims[i];
if (!isChainRegistered[_claim.chainID]) {
revert ChainIsNotRegistered(_claim.chainID);
}
Expand All @@ -120,7 +120,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
_submitClaimsBEFC(_claims, i, _caller);
}
for (uint i = 0; i < _claims.refundRequestClaims.length; i++) {
RefundRequestClaim memory _claim = _claims.refundRequestClaims[i];
RefundRequestClaim calldata _claim = _claims.refundRequestClaims[i];
if (!isChainRegistered[_claim.chainID]) {
revert ChainIsNotRegistered(_claim.chainID);
}
Expand All @@ -136,7 +136,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
_submitClaimsRRC(_claims, i, _caller);
}
for (uint i = 0; i < _claims.refundExecutedClaims.length; i++) {
RefundExecutedClaim memory _claim = _claims.refundExecutedClaims[i];
RefundExecutedClaim calldata _claim = _claims.refundExecutedClaims[i];
if (!isChainRegistered[_claim.chainID]) {
revert ChainIsNotRegistered(_claim.chainID);
}
Expand All @@ -154,7 +154,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}

function _submitClaimsBRC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
BridgingRequestClaim memory _claim = _claims.bridgingRequestClaims[index];
BridgingRequestClaim calldata _claim = _claims.bridgingRequestClaims[index];
bytes32 claimHash = keccak256(abi.encode(_claim));
uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);

Expand All @@ -180,7 +180,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}

function _submitClaimsBEC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
BatchExecutedClaim memory _claim = _claims.batchExecutedClaims[index];
BatchExecutedClaim calldata _claim = _claims.batchExecutedClaims[index];
bytes32 claimHash = keccak256(abi.encode(_claim));
uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);

Expand All @@ -206,7 +206,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}

function _submitClaimsBEFC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
BatchExecutionFailedClaim memory _claim = _claims.batchExecutionFailedClaims[index];
BatchExecutionFailedClaim calldata _claim = _claims.batchExecutionFailedClaims[index];
bytes32 claimHash = keccak256(abi.encode(_claim));
uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);

Expand All @@ -220,7 +220,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}

function _submitClaimsRRC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
RefundRequestClaim memory _claim = _claims.refundRequestClaims[index];
RefundRequestClaim calldata _claim = _claims.refundRequestClaims[index];
bytes32 claimHash = keccak256(abi.encode(_claim));
uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);

Expand All @@ -230,7 +230,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}

function _submitClaimsREC(ValidatorClaims calldata _claims, uint256 index, address _caller) internal {
RefundExecutedClaim memory _claim = _claims.refundExecutedClaims[index];
RefundExecutedClaim calldata _claim = _claims.refundExecutedClaims[index];
bytes32 claimHash = keccak256(abi.encode(_claim));
uint256 votesCnt = claimsHelper.setVoted(_claim.observedTransactionHash, _caller, claimHash);

Expand All @@ -239,8 +239,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}
}

function _setConfirmedTransactions(BridgingRequestClaim memory _claim) internal {
// passed the claim with the memory keyword
function _setConfirmedTransactions(BridgingRequestClaim calldata _claim) internal {
uint256 nextNonce = ++lastConfirmedTxNonce[_claim.destinationChainID];
confirmedTransactions[_claim.destinationChainID][nextNonce].observedTransactionHash = _claim
.observedTransactionHash;
Expand Down Expand Up @@ -273,13 +272,13 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}

function getConfirmedTransaction(
string memory _destinationChain,
string calldata _destinationChain,
uint256 _nonce
) public view returns (ConfirmedTransaction memory) {
return confirmedTransactions[_destinationChain][_nonce];
}

function getBatchingTxsCount(string memory _chainId) public view returns (uint256 counterConfirmedTransactions) {
function getBatchingTxsCount(string calldata _chainId) public view returns (uint256 counterConfirmedTransactions) {
uint256 lastConfirmedTxNonceForChain = lastConfirmedTxNonce[_chainId];
uint256 lastBatchedTxNonceForChain = lastBatchedTxNonce[_chainId];

Expand All @@ -301,7 +300,7 @@ contract Claims is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}

function getTokenAmountFromSignedBatch(
string memory _destinationChain,
string calldata _destinationChain,
uint256 _nonce
) public view returns (uint256) {
uint256 bridgedAmount;
Expand Down
18 changes: 0 additions & 18 deletions contracts/ClaimsHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,6 @@ contract ClaimsHelper is IBridgeStructs, Initializable, OwnableUpgradeable, UUPS
return v;
}

function _equal(string memory a, string memory b) internal pure returns (bool) {
return bytes(a).length == bytes(b).length && keccak256(bytes(a)) == keccak256(bytes(b));
}

function _equalReveivers(Receiver[] memory a, Receiver[] memory b) internal pure returns (bool) {
if (a.length != b.length) {
return false;
}

for (uint256 i = 0; i < a.length; i++) {
if (a[i].amount != b[i].amount || !_equal(a[i].destinationAddress, b[i].destinationAddress)) {
return false;
}
}

return true;
}

modifier onlySignedBatchesOrClaims() {
if (msg.sender != signedBatchesAddress && msg.sender != claimsAddress) revert NotSignedBatchesOrBridge();
_;
Expand Down
2 changes: 1 addition & 1 deletion contracts/Slots.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract Slots is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrade
) external onlyBridge {
// Check if the caller has already voted for this claim
for (uint i = 0; i < blocks.length; i++) {
CardanoBlock memory cblock = blocks[i];
CardanoBlock calldata cblock = blocks[i];
bytes32 chash = keccak256(abi.encodePacked(cblock.blockHash, cblock.blockSlot));
if (slotValidatorVotedPerChain[chainID][chash][_caller]) {
continue;
Expand Down
6 changes: 3 additions & 3 deletions contracts/UTXOsc.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ contract UTXOsc is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
claimsAddress = _claimsAddress;
}

function getChainUTXOs(string memory _chainID) external view returns (UTXOs memory) {
function getChainUTXOs(string calldata _chainID) external view returns (UTXOs memory) {
return chainUTXOs[_chainID];
}

Expand All @@ -51,7 +51,7 @@ contract UTXOsc is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
_removeFeeUTXOs(_chainID, _utxos.feePayerOwnedUTXOs);
}

function _removeMultisigUTXOs(string calldata _chainID, UTXO[] memory utxos) internal {
function _removeMultisigUTXOs(string calldata _chainID, UTXO[] calldata utxos) internal {
uint i = 0;
uint lenu = chainUTXOs[_chainID].multisigOwnedUTXOs.length;
while (i < lenu) {
Expand All @@ -74,7 +74,7 @@ contract UTXOsc is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUpgrad
}
}

function _removeFeeUTXOs(string calldata _chainID, UTXO[] memory utxos) internal {
function _removeFeeUTXOs(string calldata _chainID, UTXO[] calldata utxos) internal {
uint lenu = chainUTXOs[_chainID].feePayerOwnedUTXOs.length;
uint i = 0;
while (i < lenu) {
Expand Down
4 changes: 2 additions & 2 deletions contracts/Validators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract Validators is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUp
_disableInitializers();
}

function initialize(address[] memory _validators) public initializer {
function initialize(address[] calldata _validators) public initializer {
__Ownable_init(msg.sender);
__UUPSUpgradeable_init();
for (uint i = 0; i < _validators.length; i++) {
Expand Down Expand Up @@ -72,7 +72,7 @@ contract Validators is IBridgeStructs, Initializable, OwnableUpgradeable, UUPSUp
}
// set validator cardano data for each validator
for (uint i = 0; i < validatorAddressCardanoData.length; i++) {
ValidatorAddressCardanoData memory dt = validatorAddressCardanoData[i];
ValidatorAddressCardanoData calldata dt = validatorAddressCardanoData[i];
validatorsCardanoDataPerAddress[_chainId][dt.addr] = dt.data;
}
_updateValidatorCardanoData(_chainId);
Expand Down
4 changes: 2 additions & 2 deletions test/Bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2086,7 +2086,7 @@ describe("Bridge Contract", function () {
});

describe("Batch creation", function () {
it("SignedBatch submition should be reverted if chain is not registered", async function () {
it("SignedBatch submition should return imediatelly if chain is not registered", async function () {
const { bridge, validators, owner, validatorClaimsBRC, UTXOs, validatorsCardanoData } = await loadFixture(
deployBridgeFixture
);
Expand Down Expand Up @@ -2158,7 +2158,7 @@ describe("Bridge Contract", function () {

await expect(
bridge.connect(validators[0]).submitSignedBatch(signedBatch_UnregisteredChain)
).to.be.revertedWithCustomError(bridge, "CanNotCreateBatchYet"); // should create batch should return false for unregistered chain
) // submitSignedBatch should return for unregistered chain
});

it("SignedBatch submition should do nothing if batch nounce is not correct", async function () {
Expand Down

0 comments on commit 90ddab7

Please sign in to comment.