PROTO-571: feat: read list membership for bytes32 from registrar #282
Conversation
ith-harvey
left a comment
ith-harvey
left a comment
There was a problem hiding this comment.
Prod code looks good here, would be nice to get some tests
src/RegistrarReader.sol
Outdated
|
|
||
| // Note: the idea is that these values would be set to 0 or 1, | ||
| // but they don't necessarily have to be, so we check if not 0 | ||
| return isSet != ZERO_WORD; |
There was a problem hiding this comment.
Don't see an issue with this but unclear why we don't just match the existing <VAR> == bytes32(uint256(1))
There was a problem hiding this comment.
Yeah, I went back and forth on this. Basically, setKey allows setting it to an arbitrary value so I wanted to handle that.
There was a problem hiding this comment.
This is the same logic currently used in the MerkleTreeBuilder contract.
There was a problem hiding this comment.
@Oighty do you expect this contract to be used predominantly by the TTG app and/or Solidity smart contracts (if we ever introduce bytes32 lists), or should it be moved into common and used more broadly by any project reading from the registrar? Ex: https://github.com/m0-foundation/wrapped-m-token/blob/main/src/WrappedMToken.sol#L259
There was a problem hiding this comment.
Good point. It probably should live elsewhere. It also may be better as an inheritable component or library that contracts can use directly instead of needing to be deployed. For example, the Portal could use it as a library instead of having an extra external call.
src/RegistrarReader.sol
Outdated
| * and bytes32 lists from the Registrar. | ||
| * @dev This contract fills a gap in the functionality of the base registrar contract by allowing | ||
| * users to treat key-value pairs on the registrar with a key format as lists. | ||
| * @author M^0 Labs |
| * @param value The bytes32 value to check for membership in `list`. | ||
| */ | ||
| function _isSetOnRegistrar(bytes32 list, bytes32 value) internal view returns (bool) { | ||
| bytes32 key = keccak256(abi.encodePacked(list, value)); |
There was a problem hiding this comment.
looks good, we may still add IN_LIST for extra standardization
| @@ -0,0 +1,84 @@ | |||
| // SPDX-License-Identifier: GPL-3.0 | |||
| pragma solidity 0.8.23; | |||
There was a problem hiding this comment.
bump up solidity version to latest
| bytes32 m2 = bytes32("m2"); | ||
| bytes32 m3 = bytes32("m3"); | ||
|
|
||
| bytes32 key1 = keccak256(abi.encodePacked(bytes32("someList"), m1)); |
There was a problem hiding this comment.
Since strings are autoconverted to bytes32 when passed as arguments we can call listContains() with the list_ as type string however when we call setKey() the key arg value must have the list string first typed as bytes32 -> bytes32("someList")
3 participants
See Linear card for motivation.
This expects the same format for
bytes32lists as currently used by the MerkleTreeBuilder. Specifically, we can use the genericsetKeyfunction on the Registrar to manage lists ofbytes32type by calculating thekeyaskeccak256(abi.encodePacked(list, account))and treating thevalueas a boolean flag (bytes32(0)orbytes32(uint256(1))).The Registrar stores these in the raw storage with the "VALUE" prefix instead of the "IN_LIST" prefix, but we can ensure they are treated the same by handling the lookups according to the method used to manage the values.
This is already done in practice for earners on Solana, which are
bytes32addresses. You can see an example governance proposal to set one of these values here.