feat: factory for setting tree in CSM's VettedGate
#91
Conversation
vgorkavenko
force-pushed
the
feat/set-csm-vetted-gate-tree
branch
from
d0245a4 to
22036b7
Compare
vgorkavenko
changed the title
VettedGateVettedGate
| string memory treeCid | ||
| ) private pure { | ||
| require(treeRoot != bytes32(0), ERROR_TREE_ROOT); | ||
| require(bytes(treeCid).length > 0, ERROR_TREE_CID); |
There was a problem hiding this comment.
There was a problem hiding this comment.
The new factory contract looks good!
I would recommend adding an integration or scenario test (using mock for VettedGate is fine while the new contracts are not yet deployed). This would serve as a usage example for the factory and help demonstrate the expected permissions setup.
| raise ValueError("Factory name cannot be empty") | ||
| if not isinstance(factory_name, str): | ||
| raise TypeError("Factory name must be a string") | ||
| if len(factory_name) > 32: |
There was a problem hiding this comment.
With this constraint, a factory named IdentifiedCommunityStakerSetTreeParams cannot be deployed, as its name exceeds the 32-character limit
There was a problem hiding this comment.
We can use ICSSetTreeParams
There was a problem hiding this comment.
That’s OK, but the name "IdentifiedCommunityStakerSetTreeParams" was used in the test and mentioned in a comment in the contract. If the deploy script doesn’t actually allow creating a factory with that name, it can be a bit misleading.
There was a problem hiding this comment.
We will remove the restriction from the deploy script
There was a problem hiding this comment.
@Psirex Added scenario test and removed the restriction
No description provided.