feat: ID-4134: Support Bootstrap Flow for Wallet Initial Transaction #74
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,3 +30,5 @@ scripts/*_output*.json | |
| .env.devnet | ||
| .env.testnet | ||
| .env.mainnet | ||
|
|
||
| lib/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,19 +4,136 @@ pragma solidity 0.8.17; | |
|
|
||
| import "./ModuleAuthUpgradable.sol"; | ||
| import "./ImageHashKey.sol"; | ||
| import "./ModuleStorage.sol"; | ||
| import "./NonceKey.sol"; | ||
| import "../../Wallet.sol"; | ||
|
|
||
| import "../../utils/LibBytes.sol"; | ||
|
|
||
| abstract contract ModuleAuthDynamic is ModuleAuthUpgradable { | ||
| using LibBytes for bytes; | ||
|
|
||
| /// @dev Struct to hold signature validation state to avoid stack too deep errors | ||
| struct SignatureValidationState { | ||
| uint256 rindex; | ||
| bytes32 imageHash; | ||
| uint256 totalWeight; | ||
| bool immutableSignerContractSigned; | ||
| } | ||
|
|
||
| bytes32 public immutable INIT_CODE_HASH; | ||
| address public immutable FACTORY; | ||
| address public immutable IMMUTABLE_SIGNER_CONTRACT; | ||
|
|
||
| constructor(address _factory, address _startupWalletImpl, address _immutableSignerContract) { | ||
| require(_immutableSignerContract != address(0), "ModuleAuthDynamic#constructor: INVALID_SIGNER_ADDRESS"); | ||
| // Build init code hash of the deployed wallets using that module | ||
| bytes32 initCodeHash = keccak256(abi.encodePacked(Wallet.creationCode, uint256(uint160(_startupWalletImpl)))); | ||
|
|
||
| INIT_CODE_HASH = initCodeHash; | ||
| FACTORY = _factory; | ||
| IMMUTABLE_SIGNER_CONTRACT = _immutableSignerContract; | ||
| } | ||
|
|
||
| /** | ||
| * @notice Verify signature and determine if image hash needs updating | ||
| * @param _hash Hashed signed message | ||
| * @param _signature Packed signature data containing threshold, flags, weights, and addresses/signatures | ||
| * @return verified True if the signature is valid and weight threshold is met | ||
| * @return needsUpdate True if the image hash needs to be stored (first transaction) | ||
| * @return imageHash The computed image hash from the signature | ||
| * | ||
| * @dev This function parses the signature, recovers signer addresses from verified signatures, and validates them. | ||
| * Only FLAG_SIGNATURE and FLAG_DYNAMIC_SIGNATURE are supported - FLAG_ADDRESS is intentionally not | ||
| * supported to prevent attackers from including addresses without providing valid signatures. | ||
| * | ||
| * For each verified signature, the extracted address is compared against IMMUTABLE_SIGNER_CONTRACT. | ||
| * If a match is found after signature verification, it is recorded. | ||
| * | ||
| * Special case: If this is the first transaction (nonce was 0, now 1 after increment), the immutable | ||
| * signer contract has provided a valid signature, AND the weight threshold is met, the signature is | ||
| * automatically validated and approved without checking the stored image hash. This allows the immutable | ||
| * signer to bootstrap the wallet on first use while preventing unauthorized bootstrap attacks. | ||
| */ | ||
| function _signatureValidationWithUpdateCheck( | ||
| bytes32 _hash, | ||
| bytes memory _signature | ||
| ) | ||
| internal view override returns (bool, bool, bytes32) | ||
| { | ||
| ( | ||
| uint16 threshold, // required threshold signature | ||
| uint256 rindex // read index | ||
| ) = _signature.readFirstUint16(); | ||
|
|
||
| SignatureValidationState memory state = SignatureValidationState({ | ||
|
Contributor
There was a problem hiding this comment. Storing these values in memory rather than on the stack will make the code more expensive to run
Author
There was a problem hiding this comment. Had to go down this path to workaround the "stack too deep" issue 🤔 Any suggestions on alternate approaches?
Contributor
There was a problem hiding this comment. Use local variables and change the FLAG_DYNAMIC_SIGNATURE block to: Doing this helps the compiler by providing scope to size.
Author
There was a problem hiding this comment. I wasn't able to resolve the stack issue with block scoping, have retained the memory variable approach for now.
Contributor
There was a problem hiding this comment. When I retried, I couldn't get that specific fix to work. I am not sure why.
Contributor
There was a problem hiding this comment. I have created a PR which takes an alternative approach to allow the variables to be on the stack rather than in memory. Approve it and merge it into this one if you are happy with the approach. |
||
| rindex: rindex, | ||
| imageHash: bytes32(uint256(threshold)), | ||
| totalWeight: 0, | ||
| immutableSignerContractSigned: false | ||
| }); | ||
|
|
||
| // Iterate until the image is completed | ||
| while (state.rindex < _signature.length) { | ||
| // Read next item type and addrWeight | ||
| uint256 flag; uint256 addrWeight; address addr; | ||
| (flag, addrWeight, state.rindex) = _signature.readUint8Uint8(state.rindex); | ||
|
|
||
| // Note: FLAG_ADDRESS is intentionally not supported in this module to prevent | ||
| // attackers from including the immutable signer address without providing a valid signature. | ||
| // Only FLAG_SIGNATURE and FLAG_DYNAMIC_SIGNATURE are allowed. | ||
| if (flag == FLAG_SIGNATURE) { | ||
| // Read single signature and recover signer | ||
| bytes memory signature; | ||
| (signature, state.rindex) = _signature.readBytes66(state.rindex); | ||
| addr = recoverSigner(_hash, signature); | ||
|
|
||
| // Accumulate total weight of the signature | ||
| state.totalWeight += addrWeight; | ||
|
|
||
| // Check if this signer is the immutable signer contract (only after signature verification) | ||
| if (addr == IMMUTABLE_SIGNER_CONTRACT) { | ||
| state.immutableSignerContractSigned = true; | ||
| } | ||
| } else if (flag == FLAG_DYNAMIC_SIGNATURE) { | ||
| // Read signer | ||
| (addr, state.rindex) = _signature.readAddress(state.rindex); | ||
|
|
||
| // Read signature size | ||
| uint256 size; | ||
| (size, state.rindex) = _signature.readUint16(state.rindex); | ||
|
|
||
| // Read dynamic size signature | ||
| bytes memory signature; | ||
| (signature, state.rindex) = _signature.readBytes(state.rindex, size); | ||
| require(isValidSignature(_hash, addr, signature), "ModuleAuthDynamic#_signatureValidation: INVALID_SIGNATURE"); | ||
|
|
||
| // Accumulate total weight of the signature | ||
| state.totalWeight += addrWeight; | ||
|
|
||
| // Check if this signer is the immutable signer contract (only after signature verification) | ||
| if (addr == IMMUTABLE_SIGNER_CONTRACT) { | ||
| state.immutableSignerContractSigned = true; | ||
| } | ||
| } else { | ||
| revert("ModuleAuthDynamic#_signatureValidation INVALID_FLAG"); | ||
| } | ||
|
|
||
| // Write weight and address to image | ||
| state.imageHash = keccak256(abi.encode(state.imageHash, addrWeight, addr)); | ||
| } | ||
|
|
||
| // Check if this is the first transaction (nonce was 0 before increment) and immutable signer contract | ||
| // has provided a valid signature. The immutable signer must have actually signed (not just be listed | ||
| // as an address) and the total weight must meet the threshold to prevent unauthorized bootstrap attacks. | ||
| // Note: _validateNonce increments the nonce before _signatureValidation is called, so we check for 1, not 0 | ||
| uint256 currentNonce = uint256(ModuleStorage.readBytes32Map(NonceKey.NONCE_KEY, bytes32(uint256(0)))); | ||
| if (currentNonce == 1 && state.immutableSignerContractSigned && state.totalWeight >= threshold) { | ||
| return (true, true, state.imageHash); | ||
| } | ||
naveen-imtb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| (bool verified, bool needsUpdate) = _isValidImage(state.imageHash); | ||
|
Contributor
There was a problem hiding this comment. The CFA will match the calculated image hash: a combination of the user key's address and the Immutable key's address. When an alternative user secret provider is used, the user's key will be different, so the address will be different, and hence the calculated image hash and then CFA will be different.
Contributor
There was a problem hiding this comment. What am I missing in my understanding?
Author
There was a problem hiding this comment. In the scenario where a user's CFA was generated with signer A but the user is now associated with signer B (e.g., due to using different identity providers on different chains), the This bootstrap flow bypasses the
Contributor
There was a problem hiding this comment. That makes sense. Thank you for the detailed explanation. |
||
| return ((state.totalWeight >= threshold && verified), needsUpdate, state.imageHash); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.