feat: contribute contracts stack #33
Conversation
kupermind
commented
Oct 22, 2024
kupermind
commented
- Contribute contracts stack.
| /// @return nonces Set of a single service multisig nonce. | ||
| function getMultisigNonces(address multisig) external view virtual returns (uint256[] memory nonces) { | ||
| nonces = new uint256[](1); | ||
| // The nonces are equal to the social off-chain activity corresponding multisig activity |
There was a problem hiding this comment.
This is the main idea of contributors activity checking. This can be then modified / extended.
| /// @author Andrey Lebedev - <[email protected]> | ||
| /// @author Tatiana Priemova - <[email protected]> | ||
| /// @author David Vilela - <[email protected]> | ||
| contract Contributors { |
There was a problem hiding this comment.
Storage contract for contributors. For now just a basic functionality, as it's proxy-implementation based.
| /// @author Andrey Lebedev - <[email protected]> | ||
| /// @author Tatiana Priemova - <[email protected]> | ||
| /// @author David Vilela - <[email protected]> | ||
| contract ContributorsProxy { |
There was a problem hiding this comment.
Full analogy with recent Karma contracts.
| // Contribute agent Id | ||
| uint256 public constant AGENT_ID = 6; | ||
| // Contributor service config hash mock | ||
| bytes32 public constant CONFIG_HASH = 0x0000000000000000000000000000000000000000000000000000000000000006; |
There was a problem hiding this comment.
These values don't really matter, but we have to use something.
There was a problem hiding this comment.
They do matter. We should upload a relevant config file and then take it's hash.
| struct ServiceInfo { | ||
| // Service Id | ||
| uint256 serviceId; | ||
| // Corresponding service multisig | ||
| address multisig; | ||
| // Staking instance address | ||
| address stakingInstance; | ||
| // Service owner address | ||
| address serviceOwner; | ||
| } |
There was a problem hiding this comment.
Why are we duplicating all this info here when we can retrieve it from service registry etc?
There was a problem hiding this comment.
This is better for faster access. The read from other contracts would take more gas. Also, there's no backward correspondence between multisig and serviceId, or between serviceId and stakingInstance.
|
|
||
| /// @dev Contributors initializer. | ||
| /// @param _manager Manager address. | ||
| function initialize(address _manager) external{ |
There was a problem hiding this comment.
What was the idea behind doing another periphery<>core design here? Isn't it overengineered and a single contract would suffice?
There was a problem hiding this comment.
The activity management logic might change in the future or become multi-dimensional. This way we update implementation without changing current contributors activity storage.
| } | ||
|
|
||
| /// @dev Sets service info for the social id. | ||
| /// @param socialId Social id. |
There was a problem hiding this comment.
what's the social id?
There was a problem hiding this comment.
In this case it's unique X handle id.
| /// @dev Sets contribute agent statues. | ||
| /// @param contributeAgents Contribute agent addresses. | ||
| /// @param statuses Corresponding whitelisting statues. | ||
| function setMechMarketplaceStatuses(address[] memory contributeAgents, bool[] memory statuses) external { |
There was a problem hiding this comment.
Not sure why mech marketplace is connected to this.. what's the puprose?
There was a problem hiding this comment.
The entity which will report the activity is the contribute service. That should be different from the owner.
And what's the manager?
I need a graphic showing the intended design
There was a problem hiding this comment.
Typo related to copy-pasting :)
There was a problem hiding this comment.
Will work on the graphics.
There was a problem hiding this comment.
| // Contribute agent Id | ||
| uint256 public constant AGENT_ID = 6; | ||
| // Contributor service config hash mock | ||
| bytes32 public constant CONFIG_HASH = 0x0000000000000000000000000000000000000000000000000000000000000006; |
There was a problem hiding this comment.
They do matter. We should upload a relevant config file and then take it's hash.
| address indexed multisig, address stakingInstance); | ||
|
|
||
| // Contribute agent Id | ||
| uint256 public constant AGENT_ID = 6; |
There was a problem hiding this comment.
We should deploy a meaningful agent id and then use it here
| // Prepare Safe multisig data | ||
| uint256 localNonce = nonce; | ||
| bytes memory data = abi.encodePacked(address(0), fallbackHandler, address(0), address(0), uint256(0), | ||
| localNonce, "0x"); |
There was a problem hiding this comment.
check if localNonce is safe as implemented
|
|
||
| /// @dev Creates and deploys a service for the contributor, and stakes it with a specified staking contract. | ||
| /// @notice The service cannot be registered again if it is currently staked. | ||
| /// @param socialId Contributor social Id. |
There was a problem hiding this comment.
Do we ensure uniqueness somehow?
There was a problem hiding this comment.
Working on it now, what's more concerning is that we need to have a proof the msg.sender is the actual socialId holder from the Ceramic somehow. This info is off-chain only for now.
| (serviceId, multisig) = _createAndDeploy(token, minStakingDeposit, numAgentInstances, threshold); | ||
|
|
||
| // Stake the service | ||
| _stake(socialId, serviceId, multisig, stakingInstance); |
There was a problem hiding this comment.
on the staking contract side we should ensure the configs are tight so that any wrong staking contract instance usage reverts here
There was a problem hiding this comment.
good point, let's use the verifier
| function getMultisigNonces(address multisig) external view virtual returns (uint256[] memory nonces) { | ||
| nonces = new uint256[](1); | ||
| // The nonces are equal to the social off-chain activity corresponding multisig activity | ||
| nonces[0] = IContributors(contributorsProxy).mapMutisigActivities(multisig); |
