Remove redundant memory usage in Checkpoints (#4540)

Co-authored-by: Ernesto García <ernestognw@gmail.com>

.changeset/brown-cooks-dress.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`Checkpoints`: Optimized checkpoint access by removing redundant memory usage.

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);

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);