EIP-4337 – 以太坊账户抽象增量审计

这份安全评估由 OpenZeppelin 准备。

目录

• 摘要
• 范围
  • 更新
• 系统概述
• 客户报告的发现
  • 检测温存储访问
  • 泄露的基础费用
  • 在验证支付者时重放
  • 自毁 EIP4337Manager
  • 回退原因轰炸
• 高严重性
  • 无效的聚合签名 [样本]
• 低严重性
  • 账户不能替换 EntryPoint [样本]
  • Gnosis safe 在签名失败时回退 [样本]
  • 不精确的时间范围 [核心]
  • 不正确或误导性的文档 [核心和样本]
  • 误导性的规范 [核心]
  • 不匹配的事件参数 [核心]
  • 缺少文档字符串 [核心和样本]
  • 缺少 require 语句中的错误消息 [核心和样本]
  • 缺少建议的函数 [样本]
  • 未初始化的实现合约 [样本]
  • 不受限制的回退原因 [核心]
  • 不安全的 ABI 编码
• 备注与附加信息
  • 声明 uint/int 为 uint256/int256 [核心和样本]
  • 文件重定位建议 [样本]
  • IAccount 继承反模式
  • 隐式大小限制 [核心]
  • 不完整的事件历史 [样本]
  • 缺乏索引参数 [核心]
  • 命名建议 [核心和样本]
  • 不一致的排序 [核心和样本]
  • 抵押大小不一致 [核心]
  • TODO 注释 [核心和样本]
  • 印刷错误 [核心和样本]
  • 未使用的导入 [样本]
  • 未使用的接口 [核心]
  • 对之前使用的“钱包”术语的引用 [样本]
• 结论
• 附录
  • 监控建议

摘要

类型: DeFi
时间范围: 2023-01-09 至 2023-01-27
语言: Solidity
总问题: 27 (23 个已解决, 4 个部分解决)
关键严重性问题: 0 (0 个已解决)
高严重性问题: 1 (1 个已解决)
中等严重性问题: 0 (0 个已解决)
低严重性问题: 12 (10 个已解决, 2 个部分解决)
备注与附加信息: 14 (12 个已解决, 2 个部分解决)

范围

EIP-4337 是一项规范，旨在向以太坊主网添加账户抽象功能，而不修改共识规则。以太坊基金会请求我们审查他们规范的最新修订和参考实现。

我们审计了 eth-infinitism/account-abstraction 仓库的 6dea6d8752f64914dd95d932f673ba0f9ff8e144 提交。

审计的合约包括：

contracts
├── bls
│   ├── BLSAccount.sol
│   ├── BLSAccountFactory.sol
│   ├── BLSSignatureAggregator.sol
│   └── IBLSAccount.sol
├── core
│   ├── BaseAccount.sol
│   ├── BasePaymaster.sol
│   ├── EntryPoint.sol
│   ├── SenderCreator.sol
│   └── StakeManager.sol
├── gnosis
│   ├── EIP4337Fallback.sol
│   ├── EIP4337Manager.sol
│   └── GnosisAccountFactory.sol
├── interfaces
│   ├── IAccount.sol
│   ├── IAggregatedAccount.sol
│   ├── IAggregator.sol
│   ├── ICreate2Deployer.sol
│   ├── IEntryPoint.sol
│   ├── IPaymaster.sol
│   ├── IStakeManager.sol
│   └── UserOperation.sol
├── samples
│   ├── DepositPaymaster.sol
│   ├── IOracle.sol
│   ├── SimpleAccount.sol
│   ├── SimpleAccountFactory.sol
│   ├── TestAggregatedAccount.sol
│   ├── TestAggregatedAccountFactory.sol
│   ├── TestSignatureAggregator.sol
│   ├── TokenPaymaster.sol
│   └── VerifyingPaymaster.sol
└── utils
    └── Exec.sol

最初 BLSHelper.sol 包含在范围内，但我们同意在审计过程中优先考虑对其的完整审查。

更新

审计后，以太坊基金会请求我们审查三个新的拉取请求：

• 拉取请求 #245 合并于提交 1b85cfb: 创建用户操作哈希缺图的规范结构，以防止不同用户操作之间的可能哈希冲突。
• 拉取请求 #247 合并于提交 19918cd: 将 nonce 独特性验证移至 EntryPoint 合约。这现在防止账户在多个操作之间重用 nonce，但新的“键”和“序列号”区分提供了一定的操作顺序灵活性。
• 拉取请求 #257 合并于提交 9b5f2e4: 为 handleOps 和 handleAggregatedOps 函数添加重入保护和新的 BeforeExecution 事件。这防止了可能出现的事件顺序混淆，若这些函数被递归调用。

作为修复审核过程的一部分，以及我们对这些更改的审核，我们审查了影响在范围内合约的拉取请求，直到提交 9b5f2e4。

系统概述

系统架构在 我们最初的审计报告 中描述，现在包含一系列重要更改。

例如，用户和支付者现在在验证操作时都可以更改 EVM 状态。这更为通用，减轻了支付者可能在未来版本中移除回退功能的需要。为支持此更改，增加了额外的存储限制（在 EIP 中描述），以确保批处理中所有验证访问非重叠的存储插槽。此外，用户操作可以将自己的验证委托给“聚合器”智能合约，这允许所有共享聚合器的操作一起被验证。聚合器遵循与支付者相同的抵押和节流规则。

为了防止可能的混淆，值得指出的是，在此上下文中，“聚合”是指任何可以有效验证独立用户操作的机制。样本 BLSSignatureAggregator 合约有效地验证多个用户操作上的 BLS 签名，但不使用标准的 BLS 签名聚合 技术，该技术产生一个单一消息的组合签名。尽管如此，该系统支持具有任意验证逻辑的账户，因此任何人都可以部署一个接受单一消息的聚合 BLS 签名的账户（例如，生产多重签名钱包）。

还有一些增量更改：

• 新创建的账户现在可以与用户选择的工厂合约一起初始化，以便在部署过程中提供更多的灵活性。
• “钱包”一词已被“账户”替换。
• 用户可以设置定义操作有效时限的时间限制。
• 发送者现在可以在批处理中拥有多个操作，只要他们也被抵押。

客户报告的发现

检测温存储访问

客户报告: 以太坊基金会在审计期间识别了这个问题。

在模拟过程中，EntryPoint 合约 调用发送者合约的视图函数，然后进行 常规验证。由于任何存储插槽的首次访问 成本高于后续访问，因此视图函数可以执行初始的“冷访问”，以允许常规验证函数使用“热访问”。如果不同的Gas费用决定验证函数是否耗尽了Gas，那么在模拟期间，验证将成功，但在链上会失败。在这种情况下，打包者将不得不为失败的交易支付费用。

更新: 已在 拉取请求 #216 中解决，并合并于提交 1f505c5. 聚合器逻辑已重新设计，这使得此问题变得过时。

泄露的基础费用

客户报告: 以太坊基金会在审计前识别了这个问题。

EIP 禁止账户 在验证期间使用 BASEFEE 操作码，以防止他们检测到何时报会离线模拟。然而，EntryPoint 合约 将所需的预资金传递给账户，这个金额 依赖基础费用，从而泄露该值。

更新: 已在 拉取请求 #171 中解决，并合并于提交 b34b7a0. 预资金金额现在使用可能的最大Gas费。

在验证支付者时重放

客户报告: 以太坊基金会在审计期间与我们分享了这个问题，随后由 leekt 报告。

VerifyingPaymaster 合约要求可信的签名者签名 用户操作的哈希。然而，签名没有明确说明。尤其是：

• 它并未锁定到特定的链或支付者。
• 它没有利用新的 时间限制选项。
• 它依赖账户的 nonce 来防止重放。如果账户能够多次处理同一个操作，则签名每次都将继续有效。

更新: 已在 拉取请求 #184 中解决，并合并于提交 48854ef.

自毁 EIP4337Manager

客户报告: 以太坊基金会在审计期间与我们分享了这个问题，随后由 leekt 报告。

EIP4337Manager 合约旨在增强 GnosisSafe 合约，通过提供一个 用户操作验证函数。安全合约（技术上讲是他们的代理）旨在使用 delegatecall 来访问此函数。

然而，任何人都可以通过 新模块配置管理器合约。由于管理器合约 继承了 GnosisSafe 功能，新的模块可以 触发任意函数调用 并可能自毁合约。这将有效地禁用所有使用它的安全模块。

更新: 已在 拉取请求 #208 中解决，并合并于提交 d92fec8.

回退原因轰炸

客户报告: 以太坊基金会在审计期间识别了这个问题。

EntryPoint 合约有四个地方可供外部函数调用，这些调用可以使用任意长度的消息进行回退，而 EntryPoint 必须将其复制到自己的内存中。每个实例都有不同的实际后果：

• 第一个实例 发生在用户操作完成后，这实际上允许操作消耗超过指定的 callGasLimit。如果在链上发生这种情况，用户（或支付者）仍会为额外消耗的Gas费付费。如果相反，整个批次回退，则 FailedOp 错误将不会返回，因此打包者将无法轻易识别出有问题的操作。
• 第二个实例 发生在用户验证失败时。此操作将无论如何被丢弃，而不会被添加到批次中，因此可以忽略。
• 第三 实例 和 第四实例 发生在支付者验证或结束用户操作时。此任一对象可能会在操作进入批次后第一次发生，并可能导致整个批次在未返回 FailedOp 错误的情况下回退。

更新: 部分通过 拉取请求 #178 的解决，并合并于提交 9c00e78. 只有用户操作的回退原因被限制。

高严重性

无效的聚合签名 [样本]

BLSSignatureAggregator 提供了一种机制，允许打包者 验证单个签名，然后再构建批次。成功的操作分组，以便打包者 可以结合他们的签名 进行离线处理，而 EntryPoint 可以 在线验证它们。然而，账户可能构造一个操作，在单个签名检查中通过，但在组合签名检查中仍然失败。

特别是，如果它公开的公钥 在单独验证期间 与在组合验证期间 使用的公钥不同，则尽管签名相同，但这两个验证将不一致。这可能发生，因为 initCode 的 最后 4 个字 与公钥不匹配（因为 initCode 有额外的数据，或者它们没有使用 预期的创建功能）。如果 用户的验证功能 （在单个签名验证期间未调用）更改了通过 getBlsPublicKey 返回的公钥，也可能会发生这种情况。

如果打包者构建了一个包含这些操作的批次，它将无法验证组合签名并将过错归因于聚合器，这将导致聚合器被限流，并且与同一聚合器有关的用户操作将不会被处理。

考虑同步这两个验证函数，以便它们都使用相同的公钥。

更新: 已在 拉取请求 #195 中解决，以及在 拉取请求 #216 的提交 268f103 中，均已合并于提交 1cc1c97 和 1f505c5 之间。

低严重性

账户不能替换 EntryPoint [样本]

注释 描述的 SimpleAccount 合约的 initialize 函数宣称应该存在一个机制来替换 EntryPoint 合约。该声明与它所描述的函数行为不符，其实没有机制在不升级整个账户的情况下替换 EntryPoint 合约。

考虑更新该注释以匹配行为，并如果希望该功能则引入一个替换 EntryPoint 合约的机制。

_更新: 已在 拉取请求 #192 中解决，并合并于提交 82685b2. 一个 @dev 注释已添加到 initialize 函数的文档字符串中，以澄清 _entryPoint 存储变量不是初始化程序的参数，因为需要升级才能更改 EntryPoint 地址。_

Gnosis safe 在签名失败时回退 [样本]

SIG_VALIDATION_FAILED 常量的文档 阐述 validateUserOp 在签名验证失败时必须返回该值而非回退。SimpleAccount 合约 正确遵循 该规范，但是在 EIP4337Manager 合约中，validateUserOp 函数 在签名验证失败时回退。这意味着 simulateValidation 函数 将回退，而不提供 ValidationResult 对象。

考虑更改逻辑，以便在遇到无效签名时，validateUserOp 返回 SIG_VALIDATION_FAILED。

更新: 已在 拉取请求 #181 中解决，并合并于提交 1dfb173.

不精确的时间范围 [核心]

EntryPoint 合约 减少操作过期时间戳，以将 0（应解释为“无过期”）转换为最大 uint64 值。然而，所有其他可能的过期值现在都少了一。出于可预测性的考量，建议仅修改 0 时间戳。

更新: 已在 拉取请求 #193 中解决，并合并于提交 973c0ac.

不正确或误导性的文档 [核心和样本]

发现代码库中的若干文档字符串和内联评论不正确或误导。特别是：

• 在 BaseAccount.sol:

  • 第 72 行: 文档字符串将 sigTimeRange 定义为“本操作的签名和时间范围”，但它包含签名有效性，而非签名本身。

• 在 BLSSignatureAggregator.sol:

  • 第 117 行: 文档字符串引用对 simulateUserOperation 的调用，但函数名应为 simulateValidation。

• 在 EIP4337Manager.sol:

  • 第 21 行: 文档字符串声明合约继承了 GnosisSafeStorage，而实际上继承的是 GnosisSafe。

• 在 EntryPoint.sol:

  • 第 180 行: 注释没有将 paymasterAndData 列为被排除在 MemoryUserOp 之外的动态字节数组之一。
  • 第 393 行: 文档字符串声明 _validatePaymasterPrepayment 验证支付者是否被抵押，但该函数并未执行此检查。

• 在 IPaymaster.sol:

  • 第 25-26 行: 文档字符串声明 validUntil 和 validAfter 时间戳长度为 4 字节，但这些是 8 字节（uint64）值。

• 在 IStakeManager.sol:

  • 第 7 行, 第 43-44 行: 本合同中的文档字符串只提及支付者的抵押，表明这只是唯一应参与抵押的实体。签名聚合器和工厂也需要要求遵循与支付者相同的规则。
  • 第 45 行: 文档字符串提到的“全局未抵押延迟秒”不再存在。
  • 第 47 行: DepositInfo 文档字符串解释变量大小的选择，使得 deposit 和 staked 适合于单个 uint256 字，然而第三个参数 stake 也是适合的。

• 在 SimpleAccount.sol:

  • 第 52 行: 注释提到了 execFromEntryPoint 函数，但该函数不再存在。
  • 第 57 行: execute 的文档字符串上说“直接从所有者调用，而不是通过 entryPoint”，但 _requireFromEntryPointOrOwner 函数允许 execute 被 EntryPoint 调用。该注释并未明确它是一个建议，或是强制执行的限制。
  • 第 75-79 行: 文档字符串与 initialize 函数不符。
  • 第 89-96 行: 文档字符串与 _requireFromEntryPointOrOwner 函数不符。

• 在 IEntryPoint.sol:

  • 第 26 行: @success 参数的顺序不正。

• 在 UserOperation.sol:

  • 第 25 行: callGasLimit 参数没有 @param 语句。

更新: 已在 拉取请求 #194 中解决，并在 拉取请求 #216 中合并，分别合并于提交 faf305e 和 1f505c5.

误导性的规范 [核心]

EIP 规定 在检测到 FailedOp 时，所有来自同一支付者的其他操作应从当前批次中移除。然而，这仅应适用于明确提及支付者的 FailedOp 错误，这意味着支付者存在过错。因无关原因而失败的操作不应惩罚其支付者。EIP 还 声明 userOp 验证不能为空调用 handleOps 方法。该限制也应适用于 handleAggregatedOps。

考虑在 EIP 中澄清这些要点。

更新： 在 拉取请求 #196 中部分解决并在 5929ff8 合并。更新后的 EIP 错误地将 EntryPoint 的 depositTo 函数称为 depositFor。

不匹配的事件参数 [core]

StakeLocked 事件指定了一个 withdrawTime 参数，但是传递的参数是 新的撤资延迟。考虑重命名事件参数以匹配其实际使用。

更新： 在 拉取请求 #197 中解决并在提交 545a15c 中合并。

缺失文档字符串 [core 和样例]

在 代码库 中，有几个地方没有文档字符串。例如：

• BLSAccount.sol 中的 第 24 行
• BLSAccount.sol 中的 第 39 行
• BLSAccount.sol 中的 第 44 行
• BLSAccount.sol 中的 第 48 行
• BLSSignatureAggregator.sol 中的 第 20 行
• BLSSignatureAggregator.sol 中的 第 48 行
• BLSSignatureAggregator.sol 中的 第 106 行
• IBLSAccount.sol 中的 第 10 行
• BasePaymaster.sol 中的 第 24 行
• BasePaymaster.sol 中的 第 29 行
• BasePaymaster.sol 中的 第 31 行
• EntryPoint.sol 中的 第 167 行
• StakeManager.sol 中的 第 18 行
• EIP4337Fallback.sol 中的 第 11 行
• GnosisAccountFactory.sol 中的 第 23 行
• IStakeManager.sol 中的 第 67 行
• UserOperation.sol 中的 第 34 行
• DepositPaymaster.sol 中的 第 73 行
• SimpleAccount.sol 中的 第 27 行
• SimpleAccount.sol 中的 第 31 行
• TestAggregatedAccount.sol 中的 第 23 行
• TestAggregatedAccount.sol 中的 第 34 行
• TestSignatureAggregator.sol 中的 第 16 行
• TestSignatureAggregator.sol 中的 第 28 行
• TestSignatureAggregator.sol 中的 第 43 行
• TokenPaymaster.sol 中的 第 40 行
• Exec.sol 中的 第 6 行

请考虑全面记录所有函数及其参数，特别是公共 API。在编写文档字符串时，可以考虑遵循 以太坊自然规范格式 (NatSpec)。

更新： 在 拉取请求 #212 中部分解决并在提交 eeb93b2 合并。对 GnosisAccountFactory.sol 的推荐更改未得到实施。

缺失的 require 语句错误消息 [core and samples]

在 代码库 中，有一些 require 语句缺少错误消息：

• BasePaymaster.sol 中的 第 105 行 的 require 语句
• DepositPaymaster.sol 中的 第 49 行 的 require 语句
• SimpleAccount.sol 中的 第 137 行 的 require 语句

请考虑在 require 语句中包含具体的信息和错误消息，以提高整体代码的清晰度，并在需要时促进故障排除。

更新： 在 拉取请求 #198 中解决并在提交 182b7d3 中合并。向 BasePaymaster.sol 和 DepositPaymaster.sol 的缺失 require 语句添加了错误消息，而在 SimpleAccount.sol 中的 require 语句因代码更改而被消除了。

缺失的推荐函数 [samples]

EIP 声明 一个聚合账户应当支持 getAggregationInfo 函数，并且该函数应返回账户的公钥，并可能还返回其他数据。然而，BLSAccount 合约中并没有包含 getAggregationInfo 函数。考虑将 the getBlsPublicKey 函数 重命名为 getAggregationInfo。

更新： 在 拉取请求 #199 中解决并在提交 12d2ac0 中合并。EIP 现在使用 getBlsPublicKey 函数作为示例。

未初始化的实现合约 [samples]

SimpleAccountFactory 创建一个新的实现合约，但并没有 初始化它。这意味着任何人都可以将实现合约初始化为其拥有者。

后果取决于使用的 OpenZeppelin 合约版本。该项目 要求 4.2 版本 及以后的版本，但 锁定版本 4.8。onlyProxy 修饰符于 4.3.2 版本引入，以保护升级机制。没有这个修饰符，拥有者 被授权 直接调用实现合约的升级函数，从而 允许他们 selfdestruct。

在锁定的版本中，实施的拥有者可以 执行任意调用 来自实现合约，但不应能干扰代理的操作。

尽管如此，为了减少攻击面，考虑限制支持的 OpenZeppelin 合约版本并 在 SimpleAccount 合约的构造函数中禁用初始化器，以防止任何人声称拥有权。

更新： 在 拉取请求 #201 中解决并在提交 4004ebf 中合并。

不受限制的 revert 原因 [core]

EntryPoint 合约可以发出一个 FailedOp 错误，其中 reason 参数提供额外的调试信息。然而，有两个位置（ 第 375 行 和 第 417 行）不受信任的合约可以提供原因，可能包含误导性的错误代码。例如，发送者 validateUserOp 函数的 revert 可能会返回 "AA90 invalid beneficiary"，在模拟时可能导致混淆。

考虑在外部提供的 revert 原因前添加唯一标识的错误代码前缀。

更新： 在 拉取请求 #200 中解决并在提交 3d8f450 中合并。

不安全的 ABI 编码

使用 abi.encodeWithSignature 或 abi.encodeWithSelector 来生成低级调用的 calldata 并不罕见。然而，第一个选项并没有从拼写错误中得到保护，第二个选项也不是类型安全。这导致这两种方法都易出错，应该被视为不安全。

在 EIP4337Manager.sol 中，有一些使用不安全的 ABI 编码的情况：

• 在 第 119 行
• 在 第 144 行

考虑用 abi.encodeCall 替换所有的 unsafe ABI 编码，abi.encodeCall 检查提供的值是否真正匹配被调用函数预期的类型，并且避免拼写错误。

请注意，与使用字符串字面量作为 abi.encodeCall 输入相关的 错误 在 0.8.13 版本中被修复，因此开发人员在早期版本的 Solidity 中使用此函数时应谨慎。

更新： 在 拉取请求 #220 中解决并在提交 c0a69bf 中合并。第一个示例是无效的建议，因为它正在编码一个错误。

注释与附加信息

声明 uint/int 为 uint256/int256 [core 和 samples]

在 代码库 中，存在多个 int 和 uint 的使用实例，而不是 int256 和 uint256。为了明确性，考虑将所有的 int 替换为 int256，而 uint 替换为 uint256。

更新： 在 拉取请求 #215 中部分解决，已在提交 998fa7d 合并。大多数实例已经处理，但仍然存在一些 uint 类型。

文件重定位建议 [samples]

为了提升对给定合同文件是包含核心、样本或测试代码的额外清晰度，请考虑以下建议移动项目文件：

• 在 samples 目录中，TestAggregatedAccount.sol、TestAggregatedAccountFactory.sol 和 TestSignatureAggregator.sol 包含类似于在 contracts/test 目录中的测试合同。考虑将这些文件移至 contracts/test 目录。
• bls 和 gnosis 目录包含样本账户实现，但不位于 samples 目录。考虑将这些项移动到 samples 目录。

更新： 在 拉取请求 #217 中解决，并在提交 f82cbbb 中合并。

IAccount 继承反模式

IAggregatorAccount 接口通过添加能力，来扩展基础的 IAccount 接口，该能力是 公开与账户相关的签名聚合器。为了添加支持处理聚合用户操作，IAccount 中的 validateUserOp 函数现在包含一个 aggregator 地址参数。与聚合器无关的账户 必须为此参数提供空地址。这在基类了解仅与派生类相关的功能时形成了一个反模式。

为了解决此案例和协议将来的增强，考虑在 validateUserOp 中用一个更通用的 extensions 参数来替代 aggregator 参数，用于指定聚合器及任何将来的账户特定扩展。

更新： 在 拉取请求 #216 中解决，并在提交 1f505c5 中合并。

隐性大小限制 [core]

BaseAccount 合约的 packSigTimeRange 函数 隐式假设 时间戳适合用 8 字节表示。考虑通过使用 uint64 参数来强制执行这一假设。

更新： 在 拉取请求 #203 中解决并在提交 fa46d5b 中合并。

不完整的事件历史 [samples]

BLSAccount 合约 在公钥更改时发出事件，但在 初始化时没有发出。为了完成事件历史，考虑在初始化时也发出事件。

更新： 在 拉取请求 #204 中解决并在提交 2600d7e 中合并。

缺少索引参数 [core]

在 SignatureAggregatorChanged 事件中的 aggregator 参数未被索引。考虑 索引事件参数，以避免阻碍离线服务搜索和筛选特定事件的任务。

更新： 在 拉取请求 #202 中解决并在提交 1633c06 中合并。

命名建议 [core 和 samples]

为了 favor 明确性和可读性，合约中有几个位置可能受益于更好的命名。我们的建议包括：

• 在 BaseAccount.sol 中：

  • packSigTimeRange 函数是内部的，但没有使用前缀“_”。考虑重命名为 _packSigTimeRange。

• 在 BasePaymaster.sol 中：

  • packSigTimeRange 函数是内部的，但没有使用前缀“_”。考虑重命名为 _packSigTimeRange。

• 在 BLSSignatureAggregator.sol 中：

  • 考虑将 所有实例 的 hashPublicKey 重命名为 publicKeyHash 以保持一致性。

• 在 EIP4337Manager.sol：- 考虑将本地变量 _msgSender 重命名为 msgSender 以保持一致性。

• 在 IAggregator.sol：

  • 考虑将 aggregateSignatures 函数的返回值从 aggregatesSignature 重命名为 aggregatedSignature。

• 在 IEntryPoint.sol：

  • ExecutionResult 错误使用了 validBefore 而不是 validUntil。为了保持一致性，考虑将参数名称改为 validUntil。
  • ReturnInfo 结构体的 文档 指示 validAfter 参数是包括在内的。考虑将其重命名为 validFrom 遍及整个代码库。
  • 在 AggregatorStakeInfo 结构体中，考虑将 actualAggregator 重命名为 aggregator （在 这里 的注释中也同样）。

• 在 SenderCreator.sol：

  • 在 createSender 函数中，考虑将 initAddress 变量重命名为 factory 以与 EntryPoint 合约 保持一致。

• 在 SimpleAccount.sol：

  • 在 addDeposit 函数中，考虑将 req 变量重命名为 success。

• 在 StakeManager.sol：

  • internalIncrementDeposit 是一个内部函数，其前缀使用“internal”而不是“_”。考虑更改为 _incrementDeposit。
  • getStakeInfo 函数是内部的但没有以“_”作为前缀。考虑将函数重命名为 _getStakeInfo。
  • 考虑将 getStakeInfo 的 addr 参数重命名为 account。
  • 考虑从StakeManager中的 所有实例 中删除 _unstakeDelaySec 的前导下划线，现在不再需要名为 unstakeDelaySec 的存储变量。

更新： 在 pull request #221 中解决，并在提交 7bd9909 中合并。

不一致的顺序 [核心与样本]

Solidity 风格指南指定了一个 推荐顺序 来布局合约文件中的元素，以便于在可预测的位置找到分组声明。在 代码库 的几个地方没有遵循此建议：

• 在 BLSAccount.sol： PublicKeyChanged 事件在两个函数之间定义。
• 在 BLSSignatureAggregator.sol：常量值 N 在两个函数之间定义。
• 在 IEntryPoint.sol：从 第 70 行 开始，错误和结构体定义与函数定义混杂。
• 在 IPaymaster.sol： PostOpMode 枚举在所有函数之后定义。
• 在 SimpleAccount.sol： _entryPoint 变量、 SimpleAccountInitialized 事件以及 onlyOwner 修饰符在多个函数定义之后定义。

为了提高项目的整体可读性，考虑标准化整个代码库的顺序，以符合 Solidity 风格指南的推荐。

更新： 在 pull request #211 中部分解决，并在提交　ca1b649 中合并。在 IEntryPoint.sol 中，错误的定义已被重新定位，但多个结构体的定义仍然夹在函数之间。

押金大小不一致 [核心]

StakeManager 允许 存款高达最大 uint112 值，但押金必须严格小于最大 uint112 值。考虑在两种情况下使用相同的最大值以保持一致性。

更新： 在 pull request #209 中解决，并在提交 419b7b0 时完成。

TODO注释 [核心和样本]

在 代码库 中发现以下 TODO 注释实例：

• 在 EntryPoint.sol:

  • 第 305 行

• 在 EIP4337Manager.sol:

  • 第 52 行

• 在 TokenPaymaster.sol:

  • 第 57 行

TODO 注释如果不正式作为问题进行跟踪，更可能被忽视并保持未解决。随着时间的推移，关于最初动机、计划和支持细节的重要信息也可能会丢失。考虑删除所有 TODO 注释实例，并在问题积压中进行跟踪。或者，考虑将每个内嵌的 TODO 链接到相应的问题积压条目。

更新： 在 pull request #218 中解决，并在提交 80d5c89 中合并。第一个例子已过时。其他两个不是 TODO，并改为“注意”。

排版错误 [核心和样本]

考虑处理以下排版错误：

• 在 BaseAccount.sol：

  • 第 70 行： “chain-id” 应为 “chain id”。
  • 第 76 行： “The an account” 应为 “If an account”。

• 在 BLSAccount.sol：

  • 第 9 行： 在此上下文中 “public-key” 应为 “public key”。
  • 第 12 行： “a BLS public” 应为 “a BLS public key”。
  • 第 19 行： “Mutable values slots” 应为 “Mutable value slots”。

• 在 BLSAccountFactory.sol：

  • 第 11 行： “Based n” 应为 “Based on”。
  • 第 27 行： 在此上下文中 “public-key” 应为 “public key”。
• 在 BLSHelper.sol：
• 第 32 行： “(x2 y2, z2)” 应为 “(x2, y2, z2)”。
• 第 137 行： “Doubles a points” 应为 “Doubles a point”。

• 在 BLSSignatureAggregator.sol：

  • 第 34 行： “to short” 应为 “too short”。
  • 第 89 行： 在此上下文中 “public-key” 应为 “public key”；在 “value” 和 “using” 之间删除 1 个空格。
  • 第 155 行：在 “stake” 和 “or” 之间删除 1 个空格。

• 在 DepositPaymaster.sol：

  • 第 14 行： “deposit” 应为 “deposits”。

• 在 EIP4337Manager.sol：

  • 第 106 行： “prevent mistaken replaceEIP4337Manager to disable” 应为 “prevents mistaken replaceEIP4337Manager from disabling”。

• 在 EntryPoint.sol：

  • 第 50 行： “into into” 应为 “index into”。
  • 第 69 行： “deliberately caused” 应为 “not deliberately caused”。
  • 第 80 行： “UserOperation” 应为 “UserOperations” 或 “user operations”。
  • 第 180 行： “except that” 应为 “except for”。
  • 第 180 行： 缺少闭合括号。
  • 第 522 行： “if it is was” 应为 “if it was”。
  • 第 552 行： “A50” 应为 “AA50”。
  • 第 560 行： “A51” 应为 “AA51”。

• 在 IAccount.sol：

  • 第 29 行： “The an account” 应为 “If an account”。

• 在 IAggregatedAccount.sol：

  • 第 9 行： “account, that support” 应为 “account that supports”。
  • 第 11 行： “valiate” 应为 “validate”。

• 在 IAggregator.sol：

  • 第 20 行： “return” 应为 “returns”。
  • 第 20 行： 句子以冒号结尾。
  • 第 23 行： 缺少闭合括号。

• 在 IEntryPoint.sol：

  • 第 118 行： “factor” 应为 “factory”。
  • 第 129 行： “factor” 应为 “factory”。

• 在 IPaymaster.sol：

  • 第 13 行： “agree” 应为 “agrees”。
  • 第 24 行： “validation,)” 应为 “validation)”。
  • 第 48 行： “Now its” 应为 “Now it’s”。

• 在 IStakeManager.sol：

  • 第 22 行： 文档字符串从第 29 行复制错误。
  • 第 51 行： “allow” 应为 “allows”。

• 在 SimpleAccount.sol：

  • 第 65 行： “transaction” 应为 “transactions”。

• 在 TestAggregatedAccount.sol：

  • 第 18 行： “Mutable values slots” 应为 “Mutable value slots”。

• 在 TestAggregatedAccountFactory.sol：

  • 第 10 行： “Based n” 应为 “Based on”。

• 在 TokenPaymaster.sol：

  • 第 11 行： “define itself” 应为 “defines itself”。
  • 第 14 行： 在 “getTokenValueOfEth” 的字符串上缺少闭合双引号。
  • 第 66 行： 句子不完整。

• 在 UserOperation.sol：

  • 第 16 行： “field hold” 应为 “field holds”。
  • 第 16 行： “paymaster-specific-data” 应为 “paymaster-specific data”；将此短语周围的引号也删除。

更新： 已在 pull request #219 中解决，并在提交 b4ce311 中合并。

未使用的导入 [样本]

在 代码库 中，以下行的导入未使用，可以删除：

• 导入 console 的 BLSSignatureAggregator.sol
• 导入 EIP4337Manager 的 EIP4337Fallback.sol
• 导入 Exec 的 GnosisAccountFactory.sol
• 导入 IAggregator 的 IAggregatedAccount.sol
• 导入 UserOperation 的 IAggregatedAccount.sol
• 导入 Ownable 的 DepositPaymaster.sol
• 导入 BaseAccount 的 TestAggregatedAccount.sol
• 导入 SimpleAccount 的 TestSignatureAggregator.sol
• 导入 console 在 TestSignatureAggregator.sol
• 导入 SimpleAccount 的 TokenPaymaster.sol

考虑删除未使用的导入，以避免引起混淆，从而降低代码库的整体清晰性和可读性。

更新： 已在 pull request #206 中解决，并在提交 e019bbd 中合并。

未使用的接口 [核心]

ICreate2Deployer.sol 导入已从 EntryPoint.sol 中删除，见 pull request #144，但是该文件仍存在于 interfaces 目录中。没有合约导入此文件。

考虑删除未使用的接口文件。

更新： 已在 pull request #205 中解决，并在提交 679ac11 中合并。

关于之前使用的“钱包”术语的引用 [样本]

在整个代码库中，已经努力将“钱包”的术语更改单为“帐户”，例如 SimpleWallet 已重命名为 SimpleAccount。然而，某些“钱包”的引用仍然存在于各个注释中：- 第 13 行 的 BLSAccountFactory.sol

• 第 9 行 的 GnosisAccountFactory.sol
• 第 12 行 的 TestAggregatedAccountFactory.sol
• 第 14 行 的 VerifyingPaymaster.sol
• 第 16 行 的 VerifyingPaymaster.sol

为避免混淆，考虑将这些“wallet”替换为“account”。

更新： 在 pull request #210 中解决，并在提交 d6a2db7 合并。

结论

发现一个高严重性问题。提出了几个更改，以改善代码的整体质量并减少攻击面。

附录

监控建议

虽然审计有助于识别潜在的安全风险，但也鼓励以太坊基金会在其操作中整合对链上合约活动和新 mempool 内活动的自动监控。持续监控已部署的合约有助于识别潜在威胁和影响生产环境的问题。在这种情况下，它也可能提供关于系统如何被使用或误用的有用信息。考虑监控以下项目：

• 用户操作中异常高或低的 gas 参数可能表明对系统的一般误解，或者可能在某些类型的交易中识别意外的经济机会。
• 在 mempool 中持续验证失败的操作或支付者可能表明对系统的误解，或者尝试进行拒绝服务攻击。
• 使用非标准账户、工厂和聚合器的交易可能会揭示有趣的用例，或者当前设计中的不必要限制。
• 在链上回滚的任何捆绑可能表明客户端存在问题，或在指定的限制中有边际情况。
• 任何参与者的权益异常低的操作可能提供关于打包者愿意接受的风险的有用见解。

• 原文链接： blog.openzeppelin.com/ei...
• 登链社区 AI 助手，为大家转译优秀英文文章，如有翻译不通的地方，还请包涵～