From 0bce9fb0dc746c2521e294d46389d551fa25e3bc Mon Sep 17 00:00:00 2001 From: Fraser Scott Date: Wed, 6 Dec 2023 12:20:04 +0000 Subject: [PATCH 1/5] refactor(store): order load function arguments [N-02] --- packages/store/src/Storage.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/store/src/Storage.sol b/packages/store/src/Storage.sol index f74f98a72b..19ca3c9be6 100644 --- a/packages/store/src/Storage.sol +++ b/packages/store/src/Storage.sol @@ -169,7 +169,7 @@ library Storage { // Store length mstore(result, length) } - load(storagePointer, length, offset, memoryPointer); + load(storagePointer, offset, memoryPointer, length); return result; } @@ -180,7 +180,7 @@ library Storage { * @param offset Offset within the storage location. * @param memoryPointer Pointer to the location in memory to append the data. */ - function load(uint256 storagePointer, uint256 length, uint256 offset, uint256 memoryPointer) internal view { + function load(uint256 storagePointer, uint256 offset, uint256 memoryPointer, uint256 length) internal view { if (offset > 0) { // Support offsets that are greater than 32 bytes by incrementing the storagePointer and decrementing the offset if (offset >= 32) { From 3446bda84fd82473ed7b9c596e7ba9372a90a689 Mon Sep 17 00:00:00 2001 From: Fraser Scott Date: Mon, 8 Jan 2024 11:33:42 +0000 Subject: [PATCH 2/5] refactor: align order in all storage function --- packages/store/gas-report.json | 102 +++++++++++------------ packages/store/src/Storage.sol | 10 +-- packages/store/test/GasStorageLoad.t.sol | 18 ++-- packages/world-modules/gas-report.json | 76 ++++++++--------- packages/world/gas-report.json | 22 ++--- 5 files changed, 114 insertions(+), 114 deletions(-) diff --git a/packages/store/gas-report.json b/packages/store/gas-report.json index 9f4e108ac7..7e1e3b9316 100644 --- a/packages/store/gas-report.json +++ b/packages/store/gas-report.json @@ -63,7 +63,7 @@ "file": "test/Callbacks.t.sol", "test": "testSetAndGet", "name": "Callbacks: set field", - "gasUsed": 56241 + "gasUsed": 56256 }, { "file": "test/Callbacks.t.sol", @@ -75,7 +75,7 @@ "file": "test/Callbacks.t.sol", "test": "testSetAndGet", "name": "Callbacks: push 1 element", - "gasUsed": 32499 + "gasUsed": 32514 }, { "file": "test/FieldLayout.t.sol", @@ -207,37 +207,37 @@ "file": "test/Gas.t.sol", "test": "testCompareStorageWriteMUD", "name": "MUD storage write (cold, 1 word)", - "gasUsed": 22339 + "gasUsed": 22354 }, { "file": "test/Gas.t.sol", "test": "testCompareStorageWriteMUD", "name": "MUD storage write (cold, 1 word, partial)", - "gasUsed": 22406 + "gasUsed": 22421 }, { "file": "test/Gas.t.sol", "test": "testCompareStorageWriteMUD", "name": "MUD storage write (cold, 10 words)", - "gasUsed": 199795 + "gasUsed": 199810 }, { "file": "test/Gas.t.sol", "test": "testCompareStorageWriteMUD", "name": "MUD storage write (warm, 1 word)", - "gasUsed": 339 + "gasUsed": 354 }, { "file": "test/Gas.t.sol", "test": "testCompareStorageWriteMUD", "name": "MUD storage write (warm, 1 word, partial)", - "gasUsed": 506 + "gasUsed": 521 }, { "file": "test/Gas.t.sol", "test": "testCompareStorageWriteMUD", "name": "MUD storage write (warm, 10 words)", - "gasUsed": 1795 + "gasUsed": 1810 }, { "file": "test/Gas.t.sol", @@ -303,7 +303,7 @@ "file": "test/GasStorageLoad.t.sol", "test": "testCompareStorageLoadMUD", "name": "MUD storage load field (warm, 1 word)", - "gasUsed": 245 + "gasUsed": 2300 }, { "file": "test/GasStorageLoad.t.sol", @@ -315,7 +315,7 @@ "file": "test/GasStorageLoad.t.sol", "test": "testCompareStorageLoadMUD", "name": "MUD storage load field (warm, 1 word, partial)", - "gasUsed": 378 + "gasUsed": 300 }, { "file": "test/GasStorageLoad.t.sol", @@ -369,7 +369,7 @@ "file": "test/KeyEncoding.t.sol", "test": "testRegisterAndGetFieldLayout", "name": "register KeyEncoding table", - "gasUsed": 719017 + "gasUsed": 719032 }, { "file": "test/Mixed.t.sol", @@ -381,13 +381,13 @@ "file": "test/Mixed.t.sol", "test": "testDeleteExternalCold", "name": "delete record from Mixed (external, cold)", - "gasUsed": 24357 + "gasUsed": 24372 }, { "file": "test/Mixed.t.sol", "test": "testDeleteInternalCold", "name": "delete record from Mixed (internal, cold)", - "gasUsed": 19153 + "gasUsed": 19168 }, { "file": "test/Mixed.t.sol", @@ -405,7 +405,7 @@ "file": "test/Mixed.t.sol", "test": "testSetGetDeleteExternal", "name": "delete record from Mixed (external, warm)", - "gasUsed": 8673 + "gasUsed": 8688 }, { "file": "test/Mixed.t.sol", @@ -423,7 +423,7 @@ "file": "test/Mixed.t.sol", "test": "testSetGetDeleteInternal", "name": "delete record from Mixed (internal, warm)", - "gasUsed": 7466 + "gasUsed": 7481 }, { "file": "test/PackedCounter.t.sol", @@ -603,13 +603,13 @@ "file": "test/Storage.t.sol", "test": "testStoreLoad", "name": "store 1 storage slot", - "gasUsed": 22339 + "gasUsed": 22354 }, { "file": "test/Storage.t.sol", "test": "testStoreLoad", "name": "store 34 bytes over 3 storage slots (with offset and safeTrail))", - "gasUsed": 23013 + "gasUsed": 23028 }, { "file": "test/Storage.t.sol", @@ -669,25 +669,25 @@ "file": "test/StoreCoreDynamic.t.sol", "test": "testPopFromSecondField", "name": "pop from field (cold, 1 slot, 1 uint32 item)", - "gasUsed": 18017 + "gasUsed": 18032 }, { "file": "test/StoreCoreDynamic.t.sol", "test": "testPopFromSecondField", "name": "pop from field (warm, 1 slot, 1 uint32 item)", - "gasUsed": 12024 + "gasUsed": 12039 }, { "file": "test/StoreCoreDynamic.t.sol", "test": "testPopFromThirdField", "name": "pop from field (cold, 2 slots, 10 uint32 items)", - "gasUsed": 15785 + "gasUsed": 15800 }, { "file": "test/StoreCoreDynamic.t.sol", "test": "testPopFromThirdField", "name": "pop from field (warm, 2 slots, 10 uint32 items)", - "gasUsed": 11792 + "gasUsed": 11807 }, { "file": "test/StoreCoreGas.t.sol", @@ -717,7 +717,7 @@ "file": "test/StoreCoreGas.t.sol", "test": "testDeleteData", "name": "delete record (complex data, 3 slots)", - "gasUsed": 8000 + "gasUsed": 8015 }, { "file": "test/StoreCoreGas.t.sol", @@ -747,7 +747,7 @@ "file": "test/StoreCoreGas.t.sol", "test": "testHooks", "name": "register subscriber", - "gasUsed": 57939 + "gasUsed": 57954 }, { "file": "test/StoreCoreGas.t.sol", @@ -759,19 +759,19 @@ "file": "test/StoreCoreGas.t.sol", "test": "testHooks", "name": "set static field on table with subscriber", - "gasUsed": 19761 + "gasUsed": 19791 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooks", "name": "delete record on table with subscriber", - "gasUsed": 18587 + "gasUsed": 18617 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooksDynamicData", "name": "register subscriber", - "gasUsed": 57939 + "gasUsed": 57954 }, { "file": "test/StoreCoreGas.t.sol", @@ -783,31 +783,31 @@ "file": "test/StoreCoreGas.t.sol", "test": "testHooksDynamicData", "name": "set (dynamic) field on table with subscriber", - "gasUsed": 24439 + "gasUsed": 24469 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooksDynamicData", "name": "delete (dynamic) record on table with subscriber", - "gasUsed": 20253 + "gasUsed": 20283 }, { "file": "test/StoreCoreGas.t.sol", "test": "testPushToDynamicField", "name": "push to field (1 slot, 1 uint32 item)", - "gasUsed": 9444 + "gasUsed": 9459 }, { "file": "test/StoreCoreGas.t.sol", "test": "testPushToDynamicField", "name": "push to field (2 slots, 10 uint32 items)", - "gasUsed": 32114 + "gasUsed": 32129 }, { "file": "test/StoreCoreGas.t.sol", "test": "testRegisterAndGetFieldLayout", "name": "StoreCore: register table", - "gasUsed": 640899 + "gasUsed": 640914 }, { "file": "test/StoreCoreGas.t.sol", @@ -873,7 +873,7 @@ "file": "test/StoreCoreGas.t.sol", "test": "testSetAndGetField", "name": "set static field (1 slot)", - "gasUsed": 31209 + "gasUsed": 31224 }, { "file": "test/StoreCoreGas.t.sol", @@ -885,7 +885,7 @@ "file": "test/StoreCoreGas.t.sol", "test": "testSetAndGetField", "name": "set static field (overlap 2 slot)", - "gasUsed": 29862 + "gasUsed": 29877 }, { "file": "test/StoreCoreGas.t.sol", @@ -897,7 +897,7 @@ "file": "test/StoreCoreGas.t.sol", "test": "testSetAndGetField", "name": "set dynamic field (1 slot, first dynamic field)", - "gasUsed": 53930 + "gasUsed": 53945 }, { "file": "test/StoreCoreGas.t.sol", @@ -909,7 +909,7 @@ "file": "test/StoreCoreGas.t.sol", "test": "testSetAndGetField", "name": "set dynamic field (1 slot, second dynamic field)", - "gasUsed": 32153 + "gasUsed": 32168 }, { "file": "test/StoreCoreGas.t.sol", @@ -945,13 +945,13 @@ "file": "test/StoreCoreGas.t.sol", "test": "testUpdateInDynamicField", "name": "update in field (1 slot, 1 uint32 item)", - "gasUsed": 8792 + "gasUsed": 8807 }, { "file": "test/StoreCoreGas.t.sol", "test": "testUpdateInDynamicField", "name": "push to field (2 slots, 6 uint64 items)", - "gasUsed": 9238 + "gasUsed": 9253 }, { "file": "test/StoreHook.t.sol", @@ -981,13 +981,13 @@ "file": "test/StoreHooks.t.sol", "test": "testOneSlot", "name": "StoreHooks: set field with one elements (cold)", - "gasUsed": 58247 + "gasUsed": 58262 }, { "file": "test/StoreHooks.t.sol", "test": "testTable", "name": "StoreHooks: set field (cold)", - "gasUsed": 58247 + "gasUsed": 58262 }, { "file": "test/StoreHooks.t.sol", @@ -999,55 +999,55 @@ "file": "test/StoreHooks.t.sol", "test": "testTable", "name": "StoreHooks: push 1 element (cold)", - "gasUsed": 12597 + "gasUsed": 12612 }, { "file": "test/StoreHooks.t.sol", "test": "testTable", "name": "StoreHooks: pop 1 element (warm)", - "gasUsed": 9931 + "gasUsed": 9946 }, { "file": "test/StoreHooks.t.sol", "test": "testTable", "name": "StoreHooks: push 1 element (warm)", - "gasUsed": 10614 + "gasUsed": 10629 }, { "file": "test/StoreHooks.t.sol", "test": "testTable", "name": "StoreHooks: update 1 element (warm)", - "gasUsed": 29840 + "gasUsed": 29855 }, { "file": "test/StoreHooks.t.sol", "test": "testTable", "name": "StoreHooks: delete record (warm)", - "gasUsed": 10410 + "gasUsed": 10425 }, { "file": "test/StoreHooks.t.sol", "test": "testTable", "name": "StoreHooks: set field (warm)", - "gasUsed": 30392 + "gasUsed": 30407 }, { "file": "test/StoreHooks.t.sol", "test": "testThreeSlots", "name": "StoreHooks: set field with three elements (cold)", - "gasUsed": 80935 + "gasUsed": 80950 }, { "file": "test/StoreHooks.t.sol", "test": "testTwoSlots", "name": "StoreHooks: set field with two elements (cold)", - "gasUsed": 80847 + "gasUsed": 80862 }, { "file": "test/StoreHooksColdLoad.t.sol", "test": "testDelete", "name": "StoreHooks: delete record (cold)", - "gasUsed": 19268 + "gasUsed": 19283 }, { "file": "test/StoreHooksColdLoad.t.sol", @@ -1071,13 +1071,13 @@ "file": "test/StoreHooksColdLoad.t.sol", "test": "testPop", "name": "StoreHooks: pop 1 element (cold)", - "gasUsed": 18366 + "gasUsed": 18381 }, { "file": "test/StoreHooksColdLoad.t.sol", "test": "testUpdate", "name": "StoreHooks: update 1 element (cold)", - "gasUsed": 20295 + "gasUsed": 20310 }, { "file": "test/StoreSwitch.t.sol", @@ -1143,7 +1143,7 @@ "file": "test/Vector2.t.sol", "test": "testRegisterAndGetFieldLayout", "name": "register Vector2 field layout", - "gasUsed": 442403 + "gasUsed": 442418 }, { "file": "test/Vector2.t.sol", diff --git a/packages/store/src/Storage.sol b/packages/store/src/Storage.sol index b1fd2074da..65f7221f88 100644 --- a/packages/store/src/Storage.sol +++ b/packages/store/src/Storage.sol @@ -27,7 +27,7 @@ library Storage { * @param data Bytes to store. */ function store(uint256 storagePointer, uint256 offset, bytes memory data) internal { - store(storagePointer, offset, Memory.dataPointer(data), data.length); + store(storagePointer, offset, data.length, Memory.dataPointer(data)); } /** @@ -37,7 +37,7 @@ library Storage { * @param memoryPointer Pointer to the start of the data in memory. * @param length Length of the data in bytes. */ - function store(uint256 storagePointer, uint256 offset, uint256 memoryPointer, uint256 length) internal { + function store(uint256 storagePointer, uint256 offset, uint256 length, uint256 memoryPointer) internal { if (offset > 0) { // Support offsets that are greater than 32 bytes by incrementing the storagePointer and decrementing the offset if (offset >= 32) { @@ -153,7 +153,7 @@ library Storage { * @param offset Offset within the storage location. * @return result The loaded bytes of data. */ - function load(uint256 storagePointer, uint256 length, uint256 offset) internal view returns (bytes memory result) { + function load(uint256 storagePointer, uint256 offset, uint256 length) internal view returns (bytes memory result) { uint256 memoryPointer; /// @solidity memory-safe-assembly assembly { @@ -169,7 +169,7 @@ library Storage { // Store length mstore(result, length) } - load(storagePointer, offset, memoryPointer, length); + load(storagePointer, offset, length, memoryPointer); return result; } @@ -180,7 +180,7 @@ library Storage { * @param offset Offset within the storage location. * @param memoryPointer Pointer to the location in memory to append the data. */ - function load(uint256 storagePointer, uint256 offset, uint256 memoryPointer, uint256 length) internal view { + function load(uint256 storagePointer, uint256 offset, uint256 length, uint256 memoryPointer) internal view { if (offset > 0) { // Support offsets that are greater than 32 bytes by incrementing the storagePointer and decrementing the offset if (offset >= 32) { diff --git a/packages/store/test/GasStorageLoad.t.sol b/packages/store/test/GasStorageLoad.t.sol index 7db60617b2..cfafdbcd65 100644 --- a/packages/store/test/GasStorageLoad.t.sol +++ b/packages/store/test/GasStorageLoad.t.sol @@ -71,38 +71,38 @@ contract GasStorageLoadTest is Test, GasReporter { bytes32 encodedFieldPartial = valuePartial; startGasReport("MUD storage load (cold, 1 word)"); - encodedSimple = Storage.load(SolidityStorage.STORAGE_SLOT_SIMPLE, encodedSimple.length, 0); + encodedSimple = Storage.load(SolidityStorage.STORAGE_SLOT_SIMPLE, 0, encodedSimple.length); endGasReport(); startGasReport("MUD storage load (cold, 1 word, partial)"); - encodedPartial = Storage.load(SolidityStorage.STORAGE_SLOT_PARTIAL, encodedPartial.length, 16); + encodedPartial = Storage.load(SolidityStorage.STORAGE_SLOT_PARTIAL, 16, encodedPartial.length); endGasReport(); startGasReport("MUD storage load (cold, 10 words)"); - encoded9Words = Storage.load(SolidityStorage.STORAGE_SLOT_BYTES, encoded9Words.length, 0); + encoded9Words = Storage.load(SolidityStorage.STORAGE_SLOT_BYTES, 0, encoded9Words.length); endGasReport(); // warm startGasReport("MUD storage load (warm, 1 word)"); - encodedSimple = Storage.load(SolidityStorage.STORAGE_SLOT_SIMPLE, encodedSimple.length, 0); + encodedSimple = Storage.load(SolidityStorage.STORAGE_SLOT_SIMPLE, 0, encodedSimple.length); endGasReport(); startGasReport("MUD storage load field (warm, 1 word)"); - encodedFieldSimple = Storage.loadField(SolidityStorage.STORAGE_SLOT_SIMPLE, encodedSimple.length, 0); + encodedFieldSimple = Storage.loadField(SolidityStorage.STORAGE_SLOT_SIMPLE, 0, encodedSimple.length); endGasReport(); startGasReport("MUD storage load (warm, 1 word, partial)"); - encodedPartial = Storage.load(SolidityStorage.STORAGE_SLOT_PARTIAL, encodedPartial.length, 16); + encodedPartial = Storage.load(SolidityStorage.STORAGE_SLOT_PARTIAL, 16, encodedPartial.length); endGasReport(); - encodedFieldPartial = Storage.loadField(SolidityStorage.STORAGE_SLOT_PARTIAL, encodedSimple.length, 16); + encodedFieldPartial = Storage.loadField(SolidityStorage.STORAGE_SLOT_PARTIAL, 16, encodedSimple.length); startGasReport("MUD storage load field (warm, 1 word, partial)"); - encodedFieldPartial = Storage.loadField(SolidityStorage.STORAGE_SLOT_PARTIAL, encodedSimple.length, 16); + encodedFieldPartial = Storage.loadField(SolidityStorage.STORAGE_SLOT_PARTIAL, 16, encodedSimple.length); endGasReport(); startGasReport("MUD storage load (warm, 10 words)"); - encoded9Words = Storage.load(SolidityStorage.STORAGE_SLOT_BYTES, encoded9Words.length, 0); + encoded9Words = Storage.load(SolidityStorage.STORAGE_SLOT_BYTES, 0, encoded9Words.length); endGasReport(); // Do something in case the optimizer removes unused assignments diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json index 9be66918a1..2e39630ee8 100644 --- a/packages/world-modules/gas-report.json +++ b/packages/world-modules/gas-report.json @@ -3,103 +3,103 @@ "file": "test/ERC20.t.sol", "test": "testApprove", "name": "approve", - "gasUsed": 114329 + "gasUsed": 114344 }, { "file": "test/ERC20.t.sol", "test": "testBurn", "name": "burn", - "gasUsed": 75866 + "gasUsed": 75896 }, { "file": "test/ERC20.t.sol", "test": "testMint", "name": "mint", - "gasUsed": 161705 + "gasUsed": 161735 }, { "file": "test/ERC20.t.sol", "test": "testTransfer", "name": "transfer", - "gasUsed": 92948 + "gasUsed": 92978 }, { "file": "test/ERC20.t.sol", "test": "testTransferFrom", "name": "transferFrom", - "gasUsed": 130250 + "gasUsed": 130295 }, { "file": "test/ERC721.t.sol", "test": "testApproveAllGas", "name": "setApprovalForAll", - "gasUsed": 113956 + "gasUsed": 113971 }, { "file": "test/ERC721.t.sol", "test": "testApproveGas", "name": "approve", - "gasUsed": 87965 + "gasUsed": 87980 }, { "file": "test/ERC721.t.sol", "test": "testBurnGas", "name": "burn", - "gasUsed": 101835 + "gasUsed": 101880 }, { "file": "test/ERC721.t.sol", "test": "testMintGas", "name": "mint", - "gasUsed": 169433 + "gasUsed": 169463 }, { "file": "test/ERC721.t.sol", "test": "testSafeMintToEOAGas", "name": "safeMint", - "gasUsed": 169704 + "gasUsed": 169734 }, { "file": "test/ERC721.t.sol", "test": "testSafeTransferFromToEOAGas", "name": "safeTransferFrom", - "gasUsed": 143638 + "gasUsed": 143698 }, { "file": "test/ERC721.t.sol", "test": "testTransferFromGas", "name": "transferFrom", - "gasUsed": 136798 + "gasUsed": 136858 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallComposite", "name": "install keys in table module", - "gasUsed": 1413089 + "gasUsed": 1413179 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallGas", "name": "install keys in table module", - "gasUsed": 1413089 + "gasUsed": 1413179 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallGas", "name": "set a record on a table with keysInTableModule installed", - "gasUsed": 158814 + "gasUsed": 158829 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallSingleton", "name": "install keys in table module", - "gasUsed": 1413089 + "gasUsed": 1413179 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookCompositeGas", "name": "install keys in table module", - "gasUsed": 1413089 + "gasUsed": 1413179 }, { "file": "test/KeysInTableModule.t.sol", @@ -111,13 +111,13 @@ "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookCompositeGas", "name": "delete a composite record on a table with keysInTableModule installed", - "gasUsed": 155729 + "gasUsed": 155864 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookGas", "name": "install keys in table module", - "gasUsed": 1413089 + "gasUsed": 1413179 }, { "file": "test/KeysInTableModule.t.sol", @@ -129,13 +129,13 @@ "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookGas", "name": "delete a record on a table with keysInTableModule installed", - "gasUsed": 84971 + "gasUsed": 85046 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testGetKeysWithValueGas", "name": "install keys with value module", - "gasUsed": 653436 + "gasUsed": 653541 }, { "file": "test/KeysWithValueModule.t.sol", @@ -153,49 +153,49 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testInstall", "name": "install keys with value module", - "gasUsed": 653436 + "gasUsed": 653541 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testInstall", "name": "set a record on a table with KeysWithValueModule installed", - "gasUsed": 135405 + "gasUsed": 135435 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "install keys with value module", - "gasUsed": 653436 + "gasUsed": 653541 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "change a record on a table with KeysWithValueModule installed", - "gasUsed": 103789 + "gasUsed": 103819 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "delete a record on a table with KeysWithValueModule installed", - "gasUsed": 36471 + "gasUsed": 36501 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "install keys with value module", - "gasUsed": 653436 + "gasUsed": 653541 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "set a field on a table with KeysWithValueModule installed", - "gasUsed": 146626 + "gasUsed": 146671 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "change a field on a table with KeysWithValueModule installed", - "gasUsed": 111385 + "gasUsed": 111430 }, { "file": "test/query.t.sol", @@ -267,31 +267,31 @@ "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromCallboundDelegation", "name": "register a callbound delegation", - "gasUsed": 118295 + "gasUsed": 118325 }, { "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromCallboundDelegation", "name": "call a system via a callbound delegation", - "gasUsed": 36688 + "gasUsed": 36703 }, { "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromSystemDelegation", "name": "register a systembound delegation", - "gasUsed": 115851 + "gasUsed": 115881 }, { "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromSystemDelegation", "name": "call a system via a systembound delegation", - "gasUsed": 33831 + "gasUsed": 33846 }, { "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromTimeboundDelegation", "name": "register a timebound delegation", - "gasUsed": 112789 + "gasUsed": 112819 }, { "file": "test/StandardDelegationsModule.t.sol", @@ -303,24 +303,24 @@ "file": "test/UniqueEntityModule.t.sol", "test": "testInstall", "name": "install unique entity module", - "gasUsed": 681732 + "gasUsed": 681852 }, { "file": "test/UniqueEntityModule.t.sol", "test": "testInstall", "name": "get a unique entity nonce (non-root module)", - "gasUsed": 51026 + "gasUsed": 51041 }, { "file": "test/UniqueEntityModule.t.sol", "test": "testInstallRoot", "name": "installRoot unique entity module", - "gasUsed": 648129 + "gasUsed": 648249 }, { "file": "test/UniqueEntityModule.t.sol", "test": "testInstallRoot", "name": "get a unique entity nonce (root module)", - "gasUsed": 51026 + "gasUsed": 51041 } ] diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json index caf93441b1..09c68697d8 100644 --- a/packages/world/gas-report.json +++ b/packages/world/gas-report.json @@ -69,7 +69,7 @@ "file": "test/World.t.sol", "test": "testCallFromUnlimitedDelegation", "name": "register an unlimited delegation", - "gasUsed": 47623 + "gasUsed": 47638 }, { "file": "test/World.t.sol", @@ -81,13 +81,13 @@ "file": "test/World.t.sol", "test": "testDeleteRecord", "name": "Delete record", - "gasUsed": 9837 + "gasUsed": 9852 }, { "file": "test/World.t.sol", "test": "testPushToDynamicField", "name": "Push data to the table", - "gasUsed": 85811 + "gasUsed": 85826 }, { "file": "test/World.t.sol", @@ -99,7 +99,7 @@ "file": "test/World.t.sol", "test": "testRegisterNamespace", "name": "Register a new namespace", - "gasUsed": 120887 + "gasUsed": 120932 }, { "file": "test/World.t.sol", @@ -111,19 +111,19 @@ "file": "test/World.t.sol", "test": "testRegisterSystem", "name": "register a system", - "gasUsed": 164238 + "gasUsed": 164283 }, { "file": "test/World.t.sol", "test": "testRegisterTable", "name": "Register a new table in the namespace", - "gasUsed": 636447 + "gasUsed": 636507 }, { "file": "test/World.t.sol", "test": "testSetField", "name": "Write data to a table field", - "gasUsed": 36881 + "gasUsed": 36896 }, { "file": "test/World.t.sol", @@ -135,25 +135,25 @@ "file": "test/WorldDynamicUpdate.t.sol", "test": "testPopFromDynamicField", "name": "pop 1 address (cold)", - "gasUsed": 25602 + "gasUsed": 25617 }, { "file": "test/WorldDynamicUpdate.t.sol", "test": "testPopFromDynamicField", "name": "pop 1 address (warm)", - "gasUsed": 12748 + "gasUsed": 12763 }, { "file": "test/WorldDynamicUpdate.t.sol", "test": "testSpliceDynamicData", "name": "update in field 1 item (cold)", - "gasUsed": 25953 + "gasUsed": 25968 }, { "file": "test/WorldDynamicUpdate.t.sol", "test": "testSpliceDynamicData", "name": "update in field 1 item (warm)", - "gasUsed": 13154 + "gasUsed": 13169 }, { "file": "test/WorldResourceId.t.sol", From fd7ffd553879aa94a5611672ae656406ac5bbbd8 Mon Sep 17 00:00:00 2001 From: Fraser Scott Date: Mon, 8 Jan 2024 11:33:52 +0000 Subject: [PATCH 3/5] chore: changelog --- .changeset/pretty-toys-rescue.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/pretty-toys-rescue.md diff --git a/.changeset/pretty-toys-rescue.md b/.changeset/pretty-toys-rescue.md new file mode 100644 index 0000000000..ffe85995a8 --- /dev/null +++ b/.changeset/pretty-toys-rescue.md @@ -0,0 +1,12 @@ +--- +"@latticexyz/store": patch +--- + +Aligned the order of function arguments in the `Storage`` library. + +```solidity +store(uint256 storagePointer, uint256 offset, bytes memory data) +store(uint256 storagePointer, uint256 offset, uint256 length, uint256 memoryPointer) +load(uint256 storagePointer, uint256 offset, uint256 length) +load(uint256 storagePointer, uint256 offset, uint256 length, uint256 memoryPointer) +``` From 650bcfa802e4339de3a4431d227dc4714a22abd4 Mon Sep 17 00:00:00 2001 From: yonada Date: Mon, 8 Jan 2024 11:39:48 +0000 Subject: [PATCH 4/5] fix: changeset tweak --- .changeset/pretty-toys-rescue.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/pretty-toys-rescue.md b/.changeset/pretty-toys-rescue.md index ffe85995a8..45878e9dcc 100644 --- a/.changeset/pretty-toys-rescue.md +++ b/.changeset/pretty-toys-rescue.md @@ -2,7 +2,7 @@ "@latticexyz/store": patch --- -Aligned the order of function arguments in the `Storage`` library. +Aligned the order of function arguments in the `Storage` library. ```solidity store(uint256 storagePointer, uint256 offset, bytes memory data) From d2ef9b6ff221d24c3f02942f1340d8b1e25866df Mon Sep 17 00:00:00 2001 From: Fraser Scott Date: Tue, 9 Jan 2024 16:33:23 +0000 Subject: [PATCH 5/5] refactor: align arguments on named arguments --- packages/store/src/StoreCore.sol | 22 +++++++++++----------- packages/store/test/Storage.t.sol | 6 +++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/store/src/StoreCore.sol b/packages/store/src/StoreCore.sol index 60e6e993c0..50b2028d7d 100644 --- a/packages/store/src/StoreCore.sol +++ b/packages/store/src/StoreCore.sol @@ -328,8 +328,8 @@ library StoreCore { Storage.store({ storagePointer: staticDataLocation, offset: 0, - memoryPointer: memoryPointer, - length: staticData.length + length: staticData.length, + memoryPointer: memoryPointer }); // Set the dynamic data if there are dynamic fields @@ -350,8 +350,8 @@ library StoreCore { Storage.store({ storagePointer: dynamicDataLocation, offset: 0, - memoryPointer: memoryPointer, - length: dynamicDataLength + length: dynamicDataLength, + memoryPointer: memoryPointer }); memoryPointer += dynamicDataLength; // move the memory pointer to the start of the next dynamic data unchecked { @@ -740,7 +740,7 @@ library StoreCore { for (uint8 i; i < numDynamicFields; i++) { uint256 dynamicDataLocation = StoreCoreInternal._getDynamicDataLocation(tableId, keyTuple, i); uint256 length = encodedLengths.atIndex(i); - Storage.load({ storagePointer: dynamicDataLocation, length: length, offset: 0, memoryPointer: memoryPointer }); + Storage.load({ storagePointer: dynamicDataLocation, offset: 0, length: length, memoryPointer: memoryPointer }); // Advance memoryPointer by the length of this dynamic field memoryPointer += length; } @@ -828,8 +828,8 @@ library StoreCore { return Storage.load({ storagePointer: StoreCoreInternal._getDynamicDataLocation(tableId, keyTuple, dynamicFieldIndex), - length: StoreCoreInternal._loadEncodedDynamicDataLength(tableId, keyTuple).atIndex(dynamicFieldIndex), - offset: 0 + offset: 0, + length: StoreCoreInternal._loadEncodedDynamicDataLength(tableId, keyTuple).atIndex(dynamicFieldIndex) }); } @@ -917,7 +917,7 @@ library StoreCore { // Get the length and storage location of the dynamic field uint256 location = StoreCoreInternal._getDynamicDataLocation(tableId, keyTuple, dynamicFieldIndex); - return Storage.load({ storagePointer: location, length: end - start, offset: start }); + return Storage.load({ storagePointer: location, offset: start, length: end - start }); } } @@ -1078,7 +1078,7 @@ library StoreCoreInternal { // Load the data from storage uint256 location = _getStaticDataLocation(tableId, keyTuple); - return Storage.load({ storagePointer: location, length: length, offset: 0 }); + return Storage.load({ storagePointer: location, offset: 0, length: length }); } /** @@ -1100,8 +1100,8 @@ library StoreCoreInternal { return Storage.load({ storagePointer: _getStaticDataLocation(tableId, keyTuple), - length: fieldLayout.atIndex(fieldIndex), - offset: _getStaticDataOffset(fieldLayout, fieldIndex) + offset: _getStaticDataOffset(fieldLayout, fieldIndex), + length: fieldLayout.atIndex(fieldIndex) }); } diff --git a/packages/store/test/Storage.t.sol b/packages/store/test/Storage.t.sol index 6d8e178a4e..6b2fb40c8f 100644 --- a/packages/store/test/Storage.t.sol +++ b/packages/store/test/Storage.t.sol @@ -59,7 +59,7 @@ contract StorageTest is Test, GasReporter { // Assert we can load the correct partial value from storage startGasReport("load 34 bytes over 3 storage slots (with offset and safeTrail))"); - bytes memory data = Storage.load({ storagePointer: storagePointer, length: 34, offset: 31 }); + bytes memory data = Storage.load({ storagePointer: storagePointer, offset: 31, length: 34 }); endGasReport(); assertEq(Bytes.slice1(data, 0), bytes1(0x01)); @@ -73,7 +73,7 @@ contract StorageTest is Test, GasReporter { vm.assume(storagePointer > 0); Storage.store({ storagePointer: uint256(storagePointer), offset: offset, data: data }); - assertEq(Storage.load({ storagePointer: uint256(storagePointer), length: data.length, offset: offset }), data); + assertEq(Storage.load({ storagePointer: uint256(storagePointer), offset: offset, length: data.length }), data); } function testStoreLoadFieldBytes32Fuzzy(bytes32 data, uint256 storagePointer, uint256 offset) public { @@ -126,7 +126,7 @@ contract StorageTest is Test, GasReporter { uint256 storagePointer = uint256(keccak256("some location")); Storage.store({ storagePointer: storagePointer, offset: 10, data: abi.encodePacked(dataAfterLoad) }); - Storage.load({ storagePointer: storagePointer, length: 10, offset: 10, memoryPointer: memoryPointer }); + Storage.load({ storagePointer: storagePointer, offset: 10, length: 10, memoryPointer: memoryPointer }); assertEq(testData, expectedData); }