From 74ef057d3ee33685ab61bbeee4240f04de133db7 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Fri, 25 Aug 2023 23:03:34 +0300 Subject: [PATCH 1/7] Remove redundant memory usage in checkpoints --- contracts/utils/structs/Checkpoints.sol | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/contracts/utils/structs/Checkpoints.sol b/contracts/utils/structs/Checkpoints.sol index 383f01af8fa..ce3a986e0c8 100644 --- a/contracts/utils/structs/Checkpoints.sol +++ b/contracts/utils/structs/Checkpoints.sol @@ -99,7 +99,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); } } @@ -127,20 +127,22 @@ library Checkpoints { 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); @@ -286,7 +288,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); } } @@ -314,20 +316,22 @@ library Checkpoints { 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); From da54fc0b93c1bf69a37635d8245864b43c3cf035 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 28 Aug 2023 13:54:38 +0300 Subject: [PATCH 2/7] Remove old comment --- contracts/utils/structs/Checkpoints.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/utils/structs/Checkpoints.sol b/contracts/utils/structs/Checkpoints.sol index ce3a986e0c8..3bab2b5dc73 100644 --- a/contracts/utils/structs/Checkpoints.sol +++ b/contracts/utils/structs/Checkpoints.sol @@ -126,7 +126,6 @@ library Checkpoints { uint256 pos = self.length; if (pos > 0) { - // Copying to memory is important here. Checkpoint224 storage last = _unsafeAccess(self, pos - 1); uint32 lastKey = last._key; uint224 lastValue = last._value; @@ -315,7 +314,6 @@ library Checkpoints { uint256 pos = self.length; if (pos > 0) { - // Copying to memory is important here. Checkpoint160 storage last = _unsafeAccess(self, pos - 1); uint96 lastKey = last._key; uint160 lastValue = last._value; From 03d713ea6fcbf0667e301ed7a0704a2930bddae2 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Sat, 26 Aug 2023 00:38:05 +0300 Subject: [PATCH 3/7] Update template --- scripts/generate/templates/Checkpoints.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/generate/templates/Checkpoints.js b/scripts/generate/templates/Checkpoints.js index 73c9ab53e76..9885362144c 100644 --- a/scripts/generate/templates/Checkpoints.js +++ b/scripts/generate/templates/Checkpoints.js @@ -116,7 +116,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}); } } @@ -147,21 +147,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._key; + ${opts.valueTypeName} lastValue = last._value; // 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); From 984fb3f75bcc16d43abd2237ea4464b90f028454 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 28 Aug 2023 14:15:56 +0300 Subject: [PATCH 4/7] Update template --- scripts/generate/templates/Checkpoints.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/generate/templates/Checkpoints.js b/scripts/generate/templates/Checkpoints.js index 9885362144c..2a799c40c64 100644 --- a/scripts/generate/templates/Checkpoints.js +++ b/scripts/generate/templates/Checkpoints.js @@ -148,8 +148,8 @@ function _insert( if (pos > 0) { ${opts.checkpointTypeName} storage last = _unsafeAccess(self, pos - 1); - ${opts.keyTypeName} lastKey = last._key; - ${opts.valueTypeName} lastValue = last._value; + ${opts.keyTypeName} lastKey = last.${opts.keyFieldName}; + ${opts.valueTypeName} lastValue = last.${opts.valueFieldName}; // Checkpoint keys must be non-decreasing. if (lastKey > key) { From 034e50724628a8a479171f0f9452b063faab9a47 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 28 Aug 2023 14:20:16 +0300 Subject: [PATCH 5/7] Add changeset --- .changeset/brown-cooks-dress.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/brown-cooks-dress.md diff --git a/.changeset/brown-cooks-dress.md b/.changeset/brown-cooks-dress.md new file mode 100644 index 00000000000..a6203928afe --- /dev/null +++ b/.changeset/brown-cooks-dress.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Checkpoints`: removed redundant memory usage From ded4b3b731306168c72b83f3c0e67c179c4ff81e Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 16 Jan 2024 16:56:20 -0600 Subject: [PATCH 6/7] Sync generate --- contracts/utils/structs/Checkpoints.sol | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/contracts/utils/structs/Checkpoints.sol b/contracts/utils/structs/Checkpoints.sol index 569be506f11..5ba6ad92d5e 100644 --- a/contracts/utils/structs/Checkpoints.sol +++ b/contracts/utils/structs/Checkpoints.sol @@ -299,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); } } @@ -326,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); From 6ad3f63180f2954e176ec02ba2bef849980a6ef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Tue, 16 Jan 2024 16:59:07 -0600 Subject: [PATCH 7/7] Update .changeset/brown-cooks-dress.md --- .changeset/brown-cooks-dress.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/brown-cooks-dress.md b/.changeset/brown-cooks-dress.md index a6203928afe..12076df6e8f 100644 --- a/.changeset/brown-cooks-dress.md +++ b/.changeset/brown-cooks-dress.md @@ -1,5 +1,5 @@ --- -'openzeppelin-solidity': minor +'openzeppelin-solidity': patch --- -`Checkpoints`: removed redundant memory usage +`Checkpoints`: Optimized checkpoint access by removing redundant memory usage.