Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document multi-issuer contracts according to NatSpec guidelines #274

Merged
merged 13 commits into from
Jan 18, 2019

Conversation

walkerq
Copy link
Contributor

@walkerq walkerq commented Jan 16, 2019

No description provided.

@@ -24,6 +24,10 @@ pragma solidity ^0.4.24;

import "./MintController.sol";

/**
* @title MasterMinter
* @dev Default implementation of the MintController contract.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notice MasterMinter uses multiple controllers to manage minters for a contract that implements the MinterManagerInterface.
@dev MasterMinter inherits all its functionality from MintController.

@@ -28,12 +28,17 @@ import "openzeppelin-solidity/contracts/math/SafeMath.sol";

/**
* @title MintController
* @dev allows control of configure/remove minter by different addresses
*
* @dev Implementation of the abstract Controller contract, in which
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notice The MintController contract manages minters for a contract that implements the MinterManagerInterface. It lets the owner designate certain addresses as controllers, and these controllers then manage the minters by adding and removing minters, as well as modifying their minting allowance. A controller may manage exactly one minter, but the same minter address may be managed by multiple controllers.
@dev MintController inherits from the Controller contract. It treats the Controller workers as minters.

*/
contract MintController is Controller {
using SafeMath for uint256;

/**
* @title MinterManagementInterface
* @dev Interface for managing the allowances of minters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dev MintController calls the minterManager to execute/record the minter management tasks, as well as to query the status of a minter address.

// can be used for managing minters with different contracts.
/**
* @title MinterManagementInterface
* @dev Interface for managing the allowances of minters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notice A contract the implements the MinterManagementInterface has external functions for adding and removing minters and modifying their allowances. An example of such a contract is the FiatTokenV1 contract that
implements USDC.

Copy link
Contributor

@mirathewhite mirathewhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to NatSpec, the @notice function describes the functional behavior observed by the caller, while @dev explain design/architectural decisions to the developer. We should use the @notice with every function. (In most cases, this means replacing the existing @dev tag with @notice). The @dev tag can be added occasionally to explain how we implemented the @notice behavior.

@walkerq
Copy link
Contributor Author

walkerq commented Jan 16, 2019

@mirathewhite Thanks for the suggestions! I added all of them.

contracts/minting/Controller.sol Show resolved Hide resolved
contracts/minting/Controller.sol Outdated Show resolved Hide resolved
contracts/minting/Controller.sol Outdated Show resolved Hide resolved
contracts/minting/MintController.sol Outdated Show resolved Hide resolved
contracts/minting/MintController.sol Outdated Show resolved Hide resolved
@mirathewhite mirathewhite merged commit fbb6cfe into circlefin:multi-issuer Jan 18, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants