diff --git a/.changeset/brown-cooks-dress.md b/.changeset/brown-cooks-dress.md new file mode 100644 index 00000000000..12076df6e8f --- /dev/null +++ b/.changeset/brown-cooks-dress.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`Checkpoints`: Optimized checkpoint access by removing redundant memory usage. diff --git a/contracts/utils/structs/Checkpoints.sol b/contracts/utils/structs/Checkpoints.sol index 6561b0d68a8..5ba6ad92d5e 100644 --- a/contracts/utils/structs/Checkpoints.sol +++ b/contracts/utils/structs/Checkpoints.sol @@ -104,7 +104,7 @@ library Checkpoints { if (pos == 0) { return (false, 0, 0); } else { - Checkpoint224 memory ckpt = _unsafeAccess(self._checkpoints, pos - 1); + Checkpoint224 storage ckpt = _unsafeAccess(self._checkpoints, pos - 1); return (true, ckpt._key, ckpt._value); } } @@ -131,21 +131,22 @@ library Checkpoints { uint256 pos = self.length; if (pos > 0) { - // Copying to memory is important here. - Checkpoint224 memory last = _unsafeAccess(self, pos - 1); + Checkpoint224 storage last = _unsafeAccess(self, pos - 1); + uint32 lastKey = last._key; + uint224 lastValue = last._value; // Checkpoint keys must be non-decreasing. - if (last._key > key) { + if (lastKey > key) { revert CheckpointUnorderedInsertion(); } // Update or push new checkpoint - if (last._key == key) { + if (lastKey == key) { _unsafeAccess(self, pos - 1)._value = value; } else { self.push(Checkpoint224({_key: key, _value: value})); } - return (last._value, value); + return (lastValue, value); } else { self.push(Checkpoint224({_key: key, _value: value})); return (0, value); @@ -298,7 +299,7 @@ library Checkpoints { if (pos == 0) { return (false, 0, 0); } else { - Checkpoint208 memory ckpt = _unsafeAccess(self._checkpoints, pos - 1); + Checkpoint208 storage ckpt = _unsafeAccess(self._checkpoints, pos - 1); return (true, ckpt._key, ckpt._value); } } @@ -325,21 +326,22 @@ library Checkpoints { uint256 pos = self.length; if (pos > 0) { - // Copying to memory is important here. - Checkpoint208 memory last = _unsafeAccess(self, pos - 1); + Checkpoint208 storage last = _unsafeAccess(self, pos - 1); + uint48 lastKey = last._key; + uint208 lastValue = last._value; // Checkpoint keys must be non-decreasing. - if (last._key > key) { + if (lastKey > key) { revert CheckpointUnorderedInsertion(); } // Update or push new checkpoint - if (last._key == key) { + if (lastKey == key) { _unsafeAccess(self, pos - 1)._value = value; } else { self.push(Checkpoint208({_key: key, _value: value})); } - return (last._value, value); + return (lastValue, value); } else { self.push(Checkpoint208({_key: key, _value: value})); return (0, value); @@ -492,7 +494,7 @@ library Checkpoints { if (pos == 0) { return (false, 0, 0); } else { - Checkpoint160 memory ckpt = _unsafeAccess(self._checkpoints, pos - 1); + Checkpoint160 storage ckpt = _unsafeAccess(self._checkpoints, pos - 1); return (true, ckpt._key, ckpt._value); } } @@ -519,21 +521,22 @@ library Checkpoints { uint256 pos = self.length; if (pos > 0) { - // Copying to memory is important here. - Checkpoint160 memory last = _unsafeAccess(self, pos - 1); + Checkpoint160 storage last = _unsafeAccess(self, pos - 1); + uint96 lastKey = last._key; + uint160 lastValue = last._value; // Checkpoint keys must be non-decreasing. - if (last._key > key) { + if (lastKey > key) { revert CheckpointUnorderedInsertion(); } // Update or push new checkpoint - if (last._key == key) { + if (lastKey == key) { _unsafeAccess(self, pos - 1)._value = value; } else { self.push(Checkpoint160({_key: key, _value: value})); } - return (last._value, value); + return (lastValue, value); } else { self.push(Checkpoint160({_key: key, _value: value})); return (0, value); diff --git a/scripts/generate/templates/Checkpoints.js b/scripts/generate/templates/Checkpoints.js index ed935f17e0a..0a677e27f07 100644 --- a/scripts/generate/templates/Checkpoints.js +++ b/scripts/generate/templates/Checkpoints.js @@ -121,7 +121,7 @@ function latestCheckpoint(${opts.historyTypeName} storage self) if (pos == 0) { return (false, 0, 0); } else { - ${opts.checkpointTypeName} memory ckpt = _unsafeAccess(self.${opts.checkpointFieldName}, pos - 1); + ${opts.checkpointTypeName} storage ckpt = _unsafeAccess(self.${opts.checkpointFieldName}, pos - 1); return (true, ckpt.${opts.keyFieldName}, ckpt.${opts.valueFieldName}); } } @@ -152,21 +152,22 @@ function _insert( uint256 pos = self.length; if (pos > 0) { - // Copying to memory is important here. - ${opts.checkpointTypeName} memory last = _unsafeAccess(self, pos - 1); + ${opts.checkpointTypeName} storage last = _unsafeAccess(self, pos - 1); + ${opts.keyTypeName} lastKey = last.${opts.keyFieldName}; + ${opts.valueTypeName} lastValue = last.${opts.valueFieldName}; // Checkpoint keys must be non-decreasing. - if(last.${opts.keyFieldName} > key) { + if (lastKey > key) { revert CheckpointUnorderedInsertion(); } // Update or push new checkpoint - if (last.${opts.keyFieldName} == key) { + if (lastKey == key) { _unsafeAccess(self, pos - 1).${opts.valueFieldName} = value; } else { self.push(${opts.checkpointTypeName}({${opts.keyFieldName}: key, ${opts.valueFieldName}: value})); } - return (last.${opts.valueFieldName}, value); + return (lastValue, value); } else { self.push(${opts.checkpointTypeName}({${opts.keyFieldName}: key, ${opts.valueFieldName}: value})); return (0, value);