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

Streamline comments and errors #528

Merged

Conversation

chapati23
Copy link
Contributor

@chapati23 chapati23 commented Oct 15, 2024

Description

Fixes https://github.com/mento-protocol/mento-general/issues/552

  • Made error messages across GoodDollar contracts more consistent
  • Made comments across GoodDollar comments more consistent
  • Introduced @inheritdoc to reduce comment duplication between interfaces and implementations in 0.8 contracts
  • Changed Broker.configureTradingLimit() from public to external

setExitContribution, createExchange, destroyExchange
are now onlyAvatar instead of onlyOwner for the
GoodDollarExchangeProvider
they didn't add any value as the mint functions
only interact with trusted contracts and tokens
using @inheritdoc for 0.8 contracts to
reduce duplicate comments
@chapati23 chapati23 force-pushed the chore/streamline-comments-and-errors branch from d962735 to 03b2275 Compare October 15, 2024 14:29
Copy link
Contributor

@baroooo baroooo left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@@ -9,6 +9,7 @@ interface IGoodDollar {

function safeTransferFrom(address from, address to, uint256 value) external;

// Only used in Fork Tests to give the Broker minting rights during testing
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if this comment makes sense since it is specific to how we use the interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, agreed, was just a stream-of-thought comment for me I think, removed

Copy link
Contributor

@bowd bowd left a comment

Choose a reason for hiding this comment

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

❤️

@chapati23 chapati23 merged commit d3a31fa into feat/GDollar-of-latest-develop Oct 18, 2024
5 checks passed
@chapati23 chapati23 deleted the chore/streamline-comments-and-errors branch October 18, 2024 10:54
bowd pushed a commit that referenced this pull request Oct 24, 2024
7704a9b chore: reserve storage gap in new contracts
040b7fe GoodDollar Fork Tests (#530)
2ee14b4 Feat(454): Exchange provider tests  (#529)
578747c test: add BancorExchangeProvider pricing tests (#532)
fcd3d02 feat: improve tests around the expansion controller (#533)
d3a31fa Streamline comments and errors (#528)
ec64f0a fix: ☦ pray to the lords of echidna
13d3a98 fix: add back forge install to the CI step
c236009 fix: echidna tests
c50a198 feat: add Broker liquidity check (#523)
914ba7d Address Slither Issues in GoodDollar/Bancor Contracts (#526)
8bcf3fd Feat/change gdollar modifiers (#524)
379a511 feat: make validate() internal
bcbd9aa chore: added "yarn todo" task to log out open todos in the code
0631c60 refactor: renamed SafeERC20 to SafeERC20MintableBurnable
2800297 feat: simplified SafeERC20
e858391 feat: extend open zeppelin's IERC20 instead of duplicating it
196f6be chore: fixed linter and compiler warnings
096efe2 test: fix fork integration test
e5308d9 feat: add GoodDollar contracts + tests
3135626 feat: update Broker + TradingLimits to 0.8 and make G$ changes
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.

3 participants