From 42e69d592f1d4a0fce55e0ee4cd538ac195586ca Mon Sep 17 00:00:00 2001 From: Eric Mastro Date: Thu, 24 Nov 2022 22:04:18 +1100 Subject: [PATCH] add user defined types for bytes32 keys and values in Mappings --- contracts/Marketplace.sol | 123 ++++++++++++++++++++---------------- contracts/libs/Debug.sol | 12 ++-- contracts/libs/Mappings.sol | 72 +++++++++++++-------- 3 files changed, 122 insertions(+), 85 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index ddc08ef..69524e1 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -55,19 +55,19 @@ contract Marketplace is Collateral, Proofs { function myRequests() public view returns (RequestId[] memory) { uint256 counter = 0; - bytes32[] storage requestIds = - activeClientRequests.getValueIds(_toBytes32(msg.sender)); - bytes32[] memory result = new bytes32[](requestIds.length); - for (uint8 i = 0; i < requestIds.length; i++) { + Mappings.ValueId[] storage valueIds = + activeClientRequests.getValueIds(Mappings.toKeyId(msg.sender)); + bytes32[] memory result = new bytes32[](valueIds.length); + for (uint8 i = 0; i < valueIds.length; i++) { // There may exist slots that are still "active", but are part of a request // that is expired but has not been set to the cancelled state yet. In that // case, return an empty array. // TODO: remove cancelled lookups https://discord.com/channels/@me/958221374076366898/1044947242013954140 - bytes32 requestId = requestIds[i]; - if (_isCancelled(RequestId.wrap(requestId))) { + Mappings.ValueId valueId = valueIds[i]; + if (_isCancelled(_toRequestId(valueId))) { continue; } - result[counter] = requestId; + result[counter] = Mappings.ValueId.unwrap(valueId); counter++; } return _toRequestIds(Utils._resize(result, counter)); @@ -84,21 +84,23 @@ contract Marketplace is Collateral, Proofs { return new SlotId[](0); } bytes32[] memory result = new bytes32[](totalSlots); - bytes32[] storage requestIds = - activeHostRequests.getValueIds(_toBytes32(msg.sender)); - for (uint256 i = 0; i < requestIds.length; i++) { + Mappings.ValueId[] storage valueIds = + activeHostRequests.getValueIds(Mappings.toKeyId(msg.sender)); + for (uint256 i = 0; i < valueIds.length; i++) { // There may exist slots that are still "active", but are part of a request // that is expired but has not been set to the cancelled state yet. In that // case, return an empty array. // TODO: remove cancelled lookups https://discord.com/channels/@me/958221374076366898/1044947242013954140 - bytes32 requestId = requestIds[i]; - if (_isCancelled(RequestId.wrap(requestId))) { + Mappings.ValueId requestId = valueIds[i]; + if (_isCancelled(_toRequestId(requestId))) { continue; } - if (activeRequestSlots.keyExists(requestIds[i])) { - bytes32[] storage slotIds = activeRequestSlots.getValueIds(requestIds[i]); + Mappings.KeyId keyId = Mappings.toKeyId(requestId); + if (activeRequestSlots.keyExists(keyId)) { + Mappings.ValueId[] storage slotIds = + activeRequestSlots.getValueIds(keyId); for (uint256 j = 0; j < slotIds.length; j++) { - result[counter] = slotIds[j]; + result[counter] = Mappings.ValueId.unwrap(slotIds[j]); counter++; } } @@ -125,11 +127,12 @@ contract Marketplace is Collateral, Proofs { context.endsAt = block.timestamp + request.ask.duration; _setProofEnd(_toEndId(id), context.endsAt); - bytes32 addrBytes32 = _toBytes32(request.client); - activeClientRequests.insert(addrBytes32, RequestId.unwrap(id)); + Mappings.KeyId addrBytes32 = Mappings.toKeyId(request.client); + activeClientRequests.insert(addrBytes32, _toValueId(id)); - if (!activeRequestSlots.keyExists(RequestId.unwrap(id))) { - activeRequestSlots.insertKey(RequestId.unwrap(id)); + Mappings.KeyId keyId = _toKeyId(id); + if (!activeRequestSlots.keyExists(keyId)) { + activeRequestSlots.insertKey(keyId); } _createLock(_toLockId(id), request.expiry); @@ -167,12 +170,12 @@ contract Marketplace is Collateral, Proofs { RequestContext storage context = _context(requestId); context.slotsFilled += 1; - bytes32 sender = _toBytes32(msg.sender); + Mappings.KeyId sender = Mappings.toKeyId(msg.sender); // address => RequestId - activeHostRequests.insert(sender, RequestId.unwrap(requestId)); + activeHostRequests.insert(sender, _toValueId(requestId)); // RequestId => SlotId - activeRequestSlots.insert(RequestId.unwrap(requestId), SlotId.unwrap(slotId)); + activeRequestSlots.insert(_toKeyId(requestId), _toValueId(slotId)); emit SlotFilled(requestId, slotIndex, slotId); if (context.slotsFilled == request.ask.slots) { @@ -200,8 +203,9 @@ contract Marketplace is Collateral, Proofs { _unexpectProofs(_toProofId(slotId)); - if (activeRequestSlots.valueExists(SlotId.unwrap(slotId))) { - activeRequestSlots.deleteValue(SlotId.unwrap(slotId)); + Mappings.ValueId valueId = _toValueId(slotId); + if (activeRequestSlots.valueExists(valueId)) { + activeRequestSlots.deleteValue(valueId); } slot.host = address(0); slot.requestId = RequestId.wrap(0); @@ -217,8 +221,8 @@ contract Marketplace is Collateral, Proofs { context.state = RequestState.Failed; _setProofEnd(_toEndId(requestId), block.timestamp - 1); context.endsAt = block.timestamp - 1; - activeClientRequests.deleteValue(RequestId.unwrap(requestId)); - activeRequestSlots.clearValues(RequestId.unwrap(requestId)); + activeClientRequests.deleteValue(_toValueId(requestId)); + activeRequestSlots.clearValues(_toKeyId(requestId)); emit RequestFailed(requestId); // TODO: burn all remaining slot collateral (note: slot collateral not @@ -235,16 +239,17 @@ contract Marketplace is Collateral, Proofs { RequestContext storage context = _context(requestId); // Request storage request = _request(requestId); context.state = RequestState.Finished; - if (activeClientRequests.valueExists(RequestId.unwrap(requestId))) { - activeClientRequests.deleteValue(RequestId.unwrap(requestId)); + Mappings.ValueId valueId = _toValueId(requestId); + if (activeClientRequests.valueExists(valueId)) { + activeClientRequests.deleteValue(valueId); } SlotId slotId = _toSlotId(requestId, slotIndex); Slot storage slot = _slot(slotId); require(!slot.hostPaid, "Already paid"); - activeRequestSlots.deleteValue(SlotId.unwrap(slotId)); + activeRequestSlots.deleteValue(_toValueId(slotId)); if (activeRequestSlots.getManyCount() == 0) { - activeRequestSlots.deleteKey(RequestId.unwrap(requestId)); - activeHostRequests.deleteValue(RequestId.unwrap(requestId)); + activeRequestSlots.deleteKey(_toKeyId(requestId)); + activeHostRequests.deleteValue(valueId); } uint256 amount = pricePerSlot(requests[requestId]); funds.sent += amount; @@ -266,9 +271,8 @@ contract Marketplace is Collateral, Proofs { // Update request state to Cancelled. Handle in the withdraw transaction // as there needs to be someone to pay for the gas to update the state context.state = RequestState.Cancelled; - activeClientRequests.deleteValue(RequestId.unwrap(requestId)); - - activeRequestSlots.clearValues(RequestId.unwrap(requestId)); + activeClientRequests.deleteValue(_toValueId(requestId)); + activeRequestSlots.clearValues(_toKeyId(requestId)); // TODO: handle dangling RequestId in activeHostRequests (for address) emit RequestCancelled(requestId); @@ -436,6 +440,14 @@ contract Marketplace is Collateral, Proofs { return RequestId.wrap(keccak256(abi.encode(request))); } + function _toRequestId(Mappings.ValueId valueId) + internal + pure + returns (RequestId) + { + return RequestId.wrap(Mappings.ValueId.unwrap(valueId)); + } + function _toRequestIds(bytes32[] memory array) private pure @@ -469,25 +481,6 @@ contract Marketplace is Collateral, Proofs { } } - function _toBytes32(address addr) - private - pure - returns (bytes32 result) - { - return bytes32(uint(uint160(addr))); - } - - function _toBytes32s(RequestId[] memory array) - private - pure - returns (bytes32[] memory result) - { - // solhint-disable-next-line no-inline-assembly - assembly { - result := array - } - } - function _toSlotId(RequestId requestId, uint256 slotIndex) internal pure @@ -508,6 +501,30 @@ contract Marketplace is Collateral, Proofs { return EndId.wrap(RequestId.unwrap(requestId)); } + function _toKeyId(RequestId requestId) + internal + pure + returns (Mappings.KeyId) + { + return Mappings.KeyId.wrap(RequestId.unwrap(requestId)); + } + + function _toValueId(RequestId requestId) + internal + pure + returns (Mappings.ValueId) + { + return Mappings.ValueId.wrap(RequestId.unwrap(requestId)); + } + + function _toValueId(SlotId slotId) + internal + pure + returns (Mappings.ValueId) + { + return Mappings.ValueId.wrap(SlotId.unwrap(slotId)); + } + function _toBytes32SetMapKey(RequestId requestId) internal pure diff --git a/contracts/libs/Debug.sol b/contracts/libs/Debug.sol index 1ddcde0..b12a075 100644 --- a/contracts/libs/Debug.sol +++ b/contracts/libs/Debug.sol @@ -39,7 +39,7 @@ library Debug { /// | | 0x002CAD0D878B163AD63EE9C098391A6F9443CD3D48916C4E63A2485F832BE57F | /// |_________________________________________________________________________________________________________________________________________| /// Referenced values: 4 - /// Unreferenced values: 1 + /// Unreferenced values: 1 (total values not deleted but are unused) /// TOTAL Values: 5 function _printTable(Mappings.Mapping storage db, string memory message) internal @@ -51,13 +51,13 @@ library Debug { console.log("| ------------------------------------------------------------------ | ------------------------------------------------------------------ |"); uint256 referencedValues = 0; for(uint8 i = 0; i < db._keyIds.length; i++) { - bytes32 keyId = db._keyIds[i]; - console.log("|", _toHex(keyId), "| |"); + Mappings.KeyId keyId = db._keyIds[i]; + console.log("|", _toHex(Mappings.KeyId.unwrap(keyId)), "| |"); - bytes32[] storage valueIds = Mappings.getValueIds(db, keyId); + Mappings.ValueId[] storage valueIds = Mappings.getValueIds(db, keyId); for(uint8 j = 0; j < valueIds.length; j++) { - bytes32 valueId = valueIds[j]; - console.log("| |", _toHex(valueId), "|"); + Mappings.ValueId valueId = valueIds[j]; + console.log("| |", _toHex(Mappings.ValueId.unwrap(valueId)), "|"); } referencedValues += valueIds.length; } diff --git a/contracts/libs/Mappings.sol b/contracts/libs/Mappings.sol index c01ba92..75d4aed 100644 --- a/contracts/libs/Mappings.sol +++ b/contracts/libs/Mappings.sol @@ -5,13 +5,16 @@ pragma solidity ^0.8.8; import "./Debug.sol"; // DELETE ME library Mappings { + type KeyId is bytes32; + type ValueId is bytes32; + // first entity is called a "One" struct Key { // needed to delete a "One" uint256 _oneListPointer; // One has many "Many" - bytes32[] _valueIds; - mapping(bytes32 => uint256) _valueIdsIndex; // valueId => row of local _valueIds + ValueId[] _valueIds; + mapping(ValueId => uint256) _valueIdsIndex; // valueId => row of local _valueIds // more app data } @@ -20,15 +23,15 @@ library Mappings { // needed to delete a "Many" uint256 _valueIdsIndex; // many has exactly one "One" - bytes32 _keyId; + KeyId _keyId; // add app fields } struct Mapping { - mapping(bytes32 => Key) _keys; - bytes32[] _keyIds; - mapping(bytes32 => Value) _values; - bytes32[] _valueIds; + mapping(KeyId => Key) _keys; + KeyId[] _keyIds; + mapping(ValueId => Value) _values; + ValueId[] _valueIds; } function keyCount(Mapping storage db) @@ -43,7 +46,7 @@ library Mappings { return db._valueIds.length; } - function getManyCount(Mapping storage db, bytes32 keyId) + function getManyCount(Mapping storage db, KeyId keyId) internal view returns(uint256 manyCount) @@ -52,48 +55,48 @@ library Mappings { return _getValueIds(db, keyId).length; } - function keyExists(Mapping storage db, bytes32 keyId) + function keyExists(Mapping storage db, KeyId keyId) internal view returns(bool) { if(keyCount(db) == 0) return false; - return db._keyIds[db._keys[keyId]._oneListPointer] == keyId; + return equals(db._keyIds[db._keys[keyId]._oneListPointer], keyId); } - function valueExists(Mapping storage db, bytes32 valueId) + function valueExists(Mapping storage db, ValueId valueId) internal view returns(bool) { if(getManyCount(db) == 0) return false; uint256 row = db._values[valueId]._valueIdsIndex; - bool retVal = db._valueIds[row] == valueId; + bool retVal = equals(db._valueIds[row], valueId); return retVal; } function _getValueIds(Mapping storage db, - bytes32 keyId) + KeyId keyId) internal view - returns(bytes32[] storage) + returns(ValueId[] storage) { require(keyExists(db, keyId), "key does not exist"); return db._keys[keyId]._valueIds; } function getValueIds(Mapping storage db, - bytes32 keyId) + KeyId keyId) internal view - returns(bytes32[] storage) + returns(ValueId[] storage) { require(keyExists(db, keyId), "key does not exist"); return _getValueIds(db, keyId); } // Insert - function insertKey(Mapping storage db, bytes32 keyId) + function insertKey(Mapping storage db, KeyId keyId) internal returns(bool) { @@ -104,7 +107,7 @@ library Mappings { return true; } - function insertValue(Mapping storage db, bytes32 keyId, bytes32 valueId) + function insertValue(Mapping storage db, KeyId keyId, ValueId valueId) internal returns(bool) { @@ -123,7 +126,7 @@ library Mappings { return true; } - function insert(Mapping storage db, bytes32 keyId, bytes32 valueId) + function insert(Mapping storage db, KeyId keyId, ValueId valueId) internal returns(bool success) { @@ -141,7 +144,7 @@ library Mappings { // Delete - function deleteKey(Mapping storage db, bytes32 keyId) + function deleteKey(Mapping storage db, KeyId keyId) internal returns(bool) { @@ -149,7 +152,7 @@ library Mappings { require(_getValueIds(db, keyId).length == 0, "references manys"); // this would break referential integrity uint256 rowToDelete = db._keys[keyId]._oneListPointer; - bytes32 keyToMove = db._keyIds[keyCount(db)-1]; + KeyId keyToMove = db._keyIds[keyCount(db)-1]; db._keyIds[rowToDelete] = keyToMove; db._keys[keyToMove]._oneListPointer = rowToDelete; db._keyIds.pop(); @@ -157,7 +160,7 @@ library Mappings { return true; } - function deleteValue(Mapping storage db, bytes32 valueId) + function deleteValue(Mapping storage db, ValueId valueId) internal returns(bool) { @@ -169,7 +172,7 @@ library Mappings { uint256 lastIndex = getManyCount(db) - 1; if (lastIndex != toDeleteIndex) { - bytes32 lastValue = db._valueIds[lastIndex]; + ValueId lastValue = db._valueIds[lastIndex]; // Move the last value to the index where the value to delete is db._valueIds[toDeleteIndex] = lastValue; @@ -178,12 +181,12 @@ library Mappings { } db._valueIds.pop(); - bytes32 keyId = db._values[valueId]._keyId; + KeyId keyId = db._values[valueId]._keyId; Key storage oneRow = db._keys[keyId]; toDeleteIndex = oneRow._valueIdsIndex[valueId]; lastIndex = oneRow._valueIds.length - 1; if (lastIndex != toDeleteIndex) { - bytes32 lastValue = oneRow._valueIds[lastIndex]; + ValueId lastValue = oneRow._valueIds[lastIndex]; // Move the last value to the index where the value to delete is oneRow._valueIds[toDeleteIndex] = lastValue; @@ -196,7 +199,7 @@ library Mappings { return true; } - function clearValues(Mapping storage db, bytes32 keyId) + function clearValues(Mapping storage db, KeyId keyId) internal returns(bool) { @@ -209,4 +212,21 @@ library Mappings { Debug._printTable(db, "[clearValues] AFTER clearing"); return result; } + + function equals(KeyId a, KeyId b) internal pure returns (bool) { + return KeyId.unwrap(a) == KeyId.unwrap(b); + } + + function equals(ValueId a, ValueId b) internal pure returns (bool) { + return ValueId.unwrap(a) == ValueId.unwrap(b); + } + + function toKeyId(address addr) internal pure returns (KeyId) { + return KeyId.wrap(bytes32(uint(uint160(addr)))); + } + + // Useful in the case where a valueId is a foreign key + function toKeyId(ValueId valueId) internal pure returns (KeyId) { + return KeyId.wrap(ValueId.unwrap(valueId)); + } }