feat(documentation): Add NatSpec comments to contracts #59
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
contracts/HubPool.sol
Outdated
| import "@openzeppelin/contracts/utils/Address.sol"; | ||
|
|
||
| /** | ||
| * @notice Contract deployed on Ethereum that houses L1 token liquidity for all SpokePools. Admin can instruct this |
Member
There was a problem hiding this comment.
should we say "admin"? this is not really correct, no? it's more that a valid bundle does the action you describe.
Contributor
There was a problem hiding this comment.
Yeah, this is definitely not an admin. Anyone can propose these and their validity is validated via the OO.
chrismaree
reviewed
Feb 28, 2022
contracts/HubPool.sol
Outdated
|
|
||
| /** | ||
| * @notice Contract deployed on Ethereum that houses L1 token liquidity for all SpokePools. Admin can instruct this | ||
| * contract to send tokens to L2 SpokePools via "pool rebalances" with which to pay out relayers on those networks. |
Member
There was a problem hiding this comment.
"with which to pay out relayers" is a bit clunky. can you refactor this sentence a bit?
chrismaree
reviewed
Feb 28, 2022
contracts/HubPool.sol
Outdated
| /** | ||
| * @notice Contract deployed on Ethereum that houses L1 token liquidity for all SpokePools. Admin can instruct this | ||
| * contract to send tokens to L2 SpokePools via "pool rebalances" with which to pay out relayers on those networks. | ||
| * Admin also can publish relayer refund and slow relay merkle roots to SpokePools via this contract. |
Member
There was a problem hiding this comment.
Again, not sure if we should use the Admin word. wdyt?
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
contracts/HubPool.sol
Outdated
| mapping(bytes32 => address) private whitelistedRoutes; | ||
|
|
||
| // Mapping of L1TokenAddress to the associated Pool information. | ||
| // Mapping of L1 token addresses to the associated pool information. |
Member
There was a problem hiding this comment.
should this comment come after the struct declaration, but before the mapping is defined?
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
contracts/HubPool.sol
Outdated
|
|
||
| function addLiquidity(address l1Token, uint256 l1TokenAmount) public payable { | ||
| /** | ||
| * @notice Deposit liquidity into this contract to earn LP fees in exchange for backstopping relays on SpokePools. |
Member
There was a problem hiding this comment.
is backstopping the right word? I get what you're saying; it totally makes sence but is it the most correct word?
Contributor
There was a problem hiding this comment.
Yeah, I think funding is probably a more correct word.
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
| * refund relayers. | ||
| * @param slowRelayRoot Slow relay root to publish to Spoke Pool where a data worker can execute leaves to | ||
| * fulfill slow relays. | ||
| */ |
Member
There was a problem hiding this comment.
you are missing a comment on poolRebalanceRoot
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
chrismaree
reviewed
Feb 28, 2022
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.