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

Release/v1.0.1 #226

Merged
merged 58 commits into from
Dec 11, 2024
Merged

Release/v1.0.1 #226

merged 58 commits into from
Dec 11, 2024

Conversation

filmakarov
Copy link
Collaborator

@filmakarov filmakarov commented Dec 10, 2024

Release v 1.0.1


PR-Codex overview

This PR focuses on updating the Nexus smart contract and related modules to support the ERC7739 standard, enhancing signature validation, and refining the deployment scripts. It also includes version updates and improvements in the testing framework.

Detailed summary

  • Removed IERC7739 interface and ERC7739Validator base contract.
  • Added support for ERC7739 and ERC7739_V1 constants in Constants.sol.
  • Updated K1Validator to return version 1.0.1.
  • Simplified library configurations in deployment scripts.
  • Enhanced signature validation to prevent malleability.
  • Added new tests for safe sender functions and ERC7739 support detection.
  • Updated README.md for deployment instructions and contract addresses.
  • Changed package name in package.json to bcnmy-nexus and version to 1.0.1.

The following files were skipped due to too many changes: yarn.lock

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

🤖 Slither Analysis Report 🔎

Slither report

# Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary
🟡 - locked-ether (1 results) (Medium)

locked-ether

🟡 Impact: Medium
🔴 Confidence: High

utils/NexusBootstrap.sol#L33-L165

constable-states

Impact: Optimization
🔴 Confidence: High

base/RegistryAdapter.sol#L10

factory/RegistryFactory.sol#L39

_This comment was automatically generated by the GitHub Actions workflow._

Copy link

Release/v1.0.1

Generated at commit: 43e1a351815f253332f0bdf3a0e2a5cb10960473

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
1
1
0
6
27
35

For more details view the full report in OpenZeppelin Code Inspector

@@ -39,6 +38,7 @@ contract Nexus is INexus, BaseAccount, ExecutionHelper, ModuleManager, UUPSUpgra
using ModeLib for ExecutionMode;
using ExecLib for bytes;
using NonceLib for uint256;
using SentinelListLib for SentinelListLib.SentinelList;

Choose a reason for hiding this comment

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

Shouldn't we be using the SentinelList4337 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in this case it is not needed as the list is in the account storage itself

// Handle potential ERC7739 support detection request
if (signature.length == 0) {
// Forces the compiler to optimize for smaller bytecode size.
if (uint256(hash) == (~signature.length / 0xffff) * 0x7739) {

Choose a reason for hiding this comment

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

wow. what's this doing?

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is to check that this is the 7739 support request

and no, we can not because we should identify that this is the request at SA level

while (next != ZERO_ADDRESS && next != SENTINEL) {
bytes4 support = IValidator(next).isValidSignatureWithSender(msg.sender, hash, signature);
if (bytes2(support) == bytes2(SUPPORTS_ERC7739) && support > result) {
result = support;

Choose a reason for hiding this comment

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

Couldn't we break the loop in the case of result = support?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we want to find the latest supported version, so no, we can't

@filmakarov filmakarov merged commit 54f4e19 into main Dec 11, 2024
6 of 8 checks passed
@filmakarov filmakarov deleted the release/v1.0.1 branch December 11, 2024 08:12
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.

5 participants