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

🔧 Enhance SmartAccount Initialization and Signature Validation #86

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

qd-qd
Copy link
Member

@qd-qd qd-qd commented Apr 15, 2024

Overview

This PR introduces enhancements to the SmartAccount contract by adding a creationFlowFuse mechanism to manage and safeguard the account creation process more effectively. These changes aim to align with EIP-7562 by ensuring the validation context remains protected and preventing potential DoS attacks by enforcing single-use validation logic during account setup.

Changes

  • Creation Flow Fuse Addition: Added a new uint96 internal variable creationFlowFuse within SmartAccount.sol. This variable acts as a fuse or flag to ensure the account creation process can only be validated once, preventing reuse and potential exploits.

  • Activation and Reset Logic: Implemented logic to set the creationFlowFuse during the initial account creation and reset it once the creation signature validation is performed. This ensures the creation process is tightly controlled and conforms to specified behaviors.

  • Validation Logic Update: Modified _validateCreationSignature method to check the creationFlowFuse status before proceeding with the signature validation. If the fuse is not set (indicating a second use), the method fails, thus enforcing single-use as intended.

  • Unit Tests: Updated and added unit tests to cover the new creation flow logic, ensuring that the fuse behaves as expected through the initialization and validation processes.

This approach not only secures the contract against misuse but also adheres strictly to the new EIP guidelines, ensuring that the contract remains robust against network-level adversarial tactics.

Links

- Add internal uint96 for tracking creation process
- Implement logic to activate and deactivate fuse
- Modify _validateCreationSignature to check and
reset creationFlowFuse ensuring single use
- Update unit tests for new creation flow logic
@qd-qd qd-qd self-assigned this Apr 15, 2024
@qd-qd qd-qd merged commit b6471ee into main Apr 15, 2024
4 checks passed
@qd-qd qd-qd deleted the fix/eip-7562-opcode-violation branch April 15, 2024 14:15
Copy link

Changes to gas cost

Generated at commit: 6b3019cc6988f9517657c4bae0a581d4f9662b45, compared to commit: 69845c99b8c96e9b7725a953f0064616112f722e

🧾 Summary (20% most significant diffs)

Contract Method Avg (+/-) %
ERC1967Proxy exposed_validateCreationSignature
extractSignerFromAuthData
+31,795 ❌
+22 ❌
+136.17%
+0.72%
SmartAccount initialize +6,131 ❌ +5.64%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
ERC1967Proxy 0 (0) exposed_validateCreationSignature
extractSignerFromAuthData
isValidSignature
35,679 (+29,368)
1,370 (+22)
32,678 (0)
+465.35%
+1.63%
0.00%
55,144 (+31,795)
3,078 (+22)
169,287 (+196)
+136.17%
+0.72%
+0.12%
58,310 (+33,734)
3,080 (+22)
45,852 (0)
+137.26%
+0.72%
0.00%
62,007 (+27,580)
3,080 (+22)
541,514 (+1,171)
+80.11%
+0.72%
+0.22%
1,542 (-253)
1,285 (+2)
12 (0)
SmartAccount 2,652,348 (+23,422) factory
initialize
351 (0)
25,272 (+22,384)
0.00%
+775.07%
1,351 (+334)
114,755 (+6,131)
+32.84%
+5.64%
1,351 (+1,000)
117,050 (+221)
+284.90%
+0.19%
2,351 (0)
117,050 (+221)
0.00%
+0.19%
2 (-1)
40 (-6)
SmartAccountHarness 2,741,692 (+23,519) initialize 117,050 (+221) +0.19% 117,050 (+221) +0.19% 117,050 (+221) +0.19% 117,050 (+221) +0.19% 5 (0)
Paymaster 1,187,223 (0) postOp
withdrawTo(address,uint256)
22,227 (0)
23,950 (0)
0.00%
0.00%
22,841 (+7)
36,932 (+61)
+0.03%
+0.17%
22,419 (-6)
24,190 (0)
-0.03%
0.00%
24,075 (0)
104,650 (+42,079)
0.00%
+67.25%
512 (0)
770 (0)
SignerVaultWebAuthnP256R1Harness 799,090 (0) exposed_get
exposed_has(bytes)
exposed_remove
exposed_set
exposed_tryGet
1,877 (0)
1,637 (0)
29,743 (0)
24,633 (0)
8,108 (-91)
0.00%
0.00%
0.00%
0.00%
-1.11%
4,896 (0)
3,655 (0)
29,950 (+4)
82,206 (-53)
8,261 (+4)
0.00%
0.00%
+0.01%
-0.06%
+0.05%
4,901 (-6)
3,660 (-6)
30,040 (0)
89,277 (-132)
8,202 (0)
-0.12%
-0.16%
0.00%
-0.15%
0.00%
7,925 (0)
5,683 (0)
30,040 (0)
89,937 (0)
8,482 (0)
0.00%
0.00%
0.00%
0.00%
0.00%
512 (0)
512 (0)
256 (0)
2,304 (0)
256 (0)
AccountFactory 1,153,485 (0) createAndInitAccount 36,307 (0) 0.00% 67,065 (+32) +0.05% 39,240 (0) 0.00% 251,525 (+221) +0.09% 296 (0)
SignerVaultVanillaP256K1TestWrapper 239,984 (0) remove
set
22,344 (0)
44,268 (0)
0.00%
0.00%
22,505 (+4)
44,429 (-4)
+0.02%
-0.01%
22,572 (0)
44,496 (0)
0.00%
0.00%
22,572 (0)
44,496 (0)
0.00%
0.00%
256 (0)
1,024 (0)
SmartAccountERC1271__EIP191 7,241,178 (+22,231)
SmartAccountERC1271__EIP712 7,268,219 (+22,231)
SmartAccountV2 2,807,426 (+23,490)
SmartAccount__ValidateCreationSignature 7,183,085 (-19,231)
SmartAccount__validateWebAuthnP256R1Signature 7,085,731 (+22,231)

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.

1 participant