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.

Sign up for GitHub

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

Jump to bottom

Add custom errors #77

Merged
merged 13 commits into from
Oct 24, 2024
Merged

Add custom errors #77

merged 13 commits into from
Oct 24, 2024

Conversation

Mike-CZ
Copy link
Collaborator

@Mike-CZ Mike-CZ commented Oct 14, 2024

This PR adds custom errors instead of require statements.

@Mike-CZ Mike-CZ requested a review from jmpike October 14, 2024 13:25
thaarok
thaarok reviewed Oct 24, 2024
View reviewed changes
Copy link
Collaborator

@thaarok thaarok left a comment

Choose a reason for hiding this comment

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

Please don't move errors outside of the contract which use them.
Creating a big single contract for errors, to be used as parent of all other contracts is not a good idea at all.

contracts/IErrors.sol Outdated Show resolved Hide resolved
contracts/IErrors.sol Outdated Show resolved Hide resolved
Base automatically changed from mike/solhint_warning to sonic_sfc October 24, 2024 08:17
Mike-CZ added 3 commits October 24, 2024 10:36
Merge branch 'sonic_sfc' into mike/custom_errors
fad15b2 
# Conflicts:
#	contracts/common/ReentrancyGuard.sol
#	contracts/ownership/Ownable.sol
#	contracts/sfc/NodeDriver.sol
#	contracts/sfc/NodeDriverAuth.sol
#	contracts/sfc/SFCBase.sol
#	contracts/sfc/SFCLib.sol
#	contracts/sfc/Updater.sol
#	contracts/test/StubEvmWriter.sol
#	contracts/test/UnitTestSFC.sol
#	test/NodeDriver.ts
#	test/SFC.ts
Remove errors interface
0bae479
Fix tests
89ca5ba
@Mike-CZ Mike-CZ requested a review from thaarok October 24, 2024 09:44
thaarok
thaarok approved these changes Oct 24, 2024
View reviewed changes
Copy link
Collaborator

@thaarok thaarok left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seems ok (at least if we suppose merging SFCBase with SFC ad SFCLib in future - otherwise we may want to reconsider sorting errors into appropriate contract)

@Mike-CZ Mike-CZ merged commit 846e556 into sonic_sfc Oct 24, 2024
2 checks passed
@Mike-CZ Mike-CZ deleted the mike/custom_errors branch October 24, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaarok thaarok thaarok approved these changes

@jmpike jmpike Awaiting requested review from jmpike

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mike-CZ @thaarok