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

PBHVerifier #74

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[submodule "pbh-verifier/lib/forge-std"]
path = pbh-verifier/lib/forge-std
url = https://github.com/foundry-rs/forge-std
[submodule "pbh-verifier/lib/world-id-contracts"]
path = pbh-verifier/lib/world-id-contracts
url = https://github.com/worldcoin/world-id-contracts
[submodule "pbh-verifier/lib/BokkyPooBahsDateTimeLibrary"]
path = pbh-verifier/lib/BokkyPooBahsDateTimeLibrary
url = https://github.com/bokkypoobah/BokkyPooBahsDateTimeLibrary
45 changes: 45 additions & 0 deletions pbh-verifier/.github/workflows/test.yml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also move this workflow to the root .github/workflows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally can we simply emit the proof instead of verifying on chain after checking the uniqueness of the nullifier hash for the builder to verify during transaction validation.

@0xOsiris could you explain your thoughts here?

Copy link
Collaborator

@0xOsiris 0xOsiris Dec 12, 2024

Choose a reason for hiding this comment

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

Additionally can we simply emit the proof instead of verifying on chain after checking the uniqueness of the nullifier hash for the builder to verify during transaction validation.

@0xOsiris could you explain your thoughts here?

I mean this is probably up for a wider discussion, but instead of verifying the proof in the PBHVerifier we can instead just emit the proof. The builder will pick up the proof(s) when validating the bundle transaction, and verify them off chain. If any proof in the transaction is invalid the bundle will be dropped from the mempool.

In this case the PBHVerifier is only in charge of storing the external nullifier hashes, and reverting the whole bundle on a rate limit. The benefit would be saving ~200,000 gas / UO, downside is higher trust assumptions in the builder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah okay I'm into that! I think this could work. Let's chat tomorrow at standup.

Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
name: CI

on:
push:
pull_request:
workflow_dispatch:

env:
FOUNDRY_PROFILE: ci

jobs:
check:
strategy:
fail-fast: true

name: Foundry project
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly

- name: Show Forge version
run: |
forge --version

- name: Run Forge fmt
run: |
forge fmt --check
id: fmt

- name: Run Forge build
run: |
forge build --sizes
id: build

- name: Run Forge tests
run: |
forge test -vvv
id: test
14 changes: 14 additions & 0 deletions pbh-verifier/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Compiler files
cache/
out/

# Ignores development broadcast logs
!/broadcast
/broadcast/*/31337/
/broadcast/**/dry-run/

# Docs
docs/

# Dotenv file
.env
7 changes: 7 additions & 0 deletions pbh-verifier/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# PBH Validator

As mentioned previously, Stage 1 of 4337 PBH features a PBHSignatureAggregator and PBHValidator contract, allowing a user to include a World ID proof encoded in the UserOp signature. The signature aggregator will call the PBHValidator for each UserOp included in the bundle, verifying the associated proof.

The `PBHValidator` contract will extract the proof data from the signature, validate proof inputs and verify the proof. The `signal` for the proof will consist of the `userOpHash`. Upon successful verification of the proof, the `PBHValidator` will bump the PBH nonce for the `nullifierHash` associated with the proof. The PBH nonce is used to ensure that a given World ID user does not use more than `n` transactions a month.

If the UserOp successfully clears all of these checks, a `PBH` event will be emitted indicating to the builder that this UserOp is a valid PBH user operation. The builder will only consider a “PBH” bundle for priority inclusion if all UserOps in the bundle emit a PBH event, and `aggregator` is specified as the `PBHSignatureAggregator`.
5 changes: 5 additions & 0 deletions pbh-verifier/foundry.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[profile.default]
src = "src"
out = "out"
libs = ["lib"]
via_ir = true
1 change: 1 addition & 0 deletions pbh-verifier/lib/BokkyPooBahsDateTimeLibrary
1 change: 1 addition & 0 deletions pbh-verifier/lib/forge-std
Submodule forge-std added at d3db4e
1 change: 1 addition & 0 deletions pbh-verifier/lib/world-id-contracts
Submodule world-id-contracts added at 583534
3 changes: 3 additions & 0 deletions pbh-verifier/remappings.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@world-id-contracts/=lib/world-id-contracts/src/
@account-abstraction/=lib/account-abstraction/contracts/
@BokkyPooBahsDateTimeLibrary/=lib/BokkyPooBahsDateTimeLibrary/contracts/
19 changes: 19 additions & 0 deletions pbh-verifier/script/Counter.s.sol.bak
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Script, console} from "forge-std/Script.sol";
import {Counter} from "../src/Counter.sol";

contract CounterScript is Script {
Counter public counter;

function setUp() public {}

function run() public {
vm.startBroadcast();

counter = new Counter();

vm.stopBroadcast();
}
}
165 changes: 165 additions & 0 deletions pbh-verifier/src/PBHVerifier.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {ByteHasher} from "./helpers/ByteHasher.sol";
import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol";
import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol";

contract PBHVerifier {
using ByteHasher for bytes;

///////////////////////////////////////////////////////////////////////////////
/// ERRORS ///
//////////////////////////////////////////////////////////////////////////////

/// @notice Thrown when attempting to reuse a nullifier
error InvalidNullifier();

/// @notice Thrown when the provided external nullifier year doesn't
/// match the current year
error InvalidExternalNullifierYear();

/// @notice Thrown when the provided external nullifier month doesn't
/// match the current month
error InvalidExternalNullifierMonth();

/// @notice Thrown when the provided external
/// nullifier pbhNonce >= numPbhPerMonth
error InvalidPbhNonce();

///////////////////////////////////////////////////////////////////////////////
/// Events ///
//////////////////////////////////////////////////////////////////////////////

/// @notice Emitted when a verifier is updated in the lookup table.
///
/// @param nullifierHash The nullifier hash that was used.
event PBH(
uint256 indexed nullifierHash
);

///////////////////////////////////////////////////////////////////////////////
/// Structs ///
//////////////////////////////////////////////////////////////////////////////

/**
* User Operation struct
* @param sender - The sender account of this request.
* @param nonce - Unique value the sender uses to verify it is not a replay.
* @param initCode - If set, the account contract will be created by this constructor/
* @param callData - The method call to execute on this account.
* @param accountGasLimits - Packed gas limits for validateUserOp and gas limit passed to the callData method call.
* @param preVerificationGas - Gas not calculated by the handleOps method, but added to the gas paid.
* Covers batch overhead.
* @param gasFees - packed gas fields maxPriorityFeePerGas and maxFeePerGas - Same as EIP-1559 gas parameters.
* @param paymasterAndData - If set, this field holds the paymaster address, verification gas limit, postOp gas limit and paymaster-specific extra data
* The paymaster will pay for the transaction instead of the sender.
* @param signature - Sender-verified signature over the entire request, the EntryPoint address and the chain ID.
*/
struct PackedUserOperation {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we inherit this type from the account-abstraction contracts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha yeah for sure :) sorry this isn't even close to being ready for review haha. I just opened the PR to show the external nullifier implementation!

address sender;
uint256 nonce;
bytes initCode;
bytes callData;
bytes32 accountGasLimits;
uint256 preVerificationGas;
bytes32 gasFees;
bytes paymasterAndData;
bytes signature;
}

struct PBHPayload {
uint256 root;
uint256 nullifierHash;
ExternalNullifier externalNullifier;
uint256[8] proof;
}

/**
* External Nullifier struct
* @param pbhNonce - A nonce between 0 and numPbhPerMonth.
* @param month - An integer representing the current month.
* @param year - An integer representing the current year.
*/
struct ExternalNullifier {
uint8 pbhNonce;
uint16 month;
uint8 year;
}

///////////////////////////////////////////////////////////////////////////////
/// Vars ///
//////////////////////////////////////////////////////////////////////////////

/// @dev The World ID instance that will be used for verifying proofs
IWorldIDGroups internal immutable worldId;

/// @dev The World ID group ID (always 1)
uint256 internal immutable groupId = 1;
Copy link
Contributor

@0xKitsune 0xKitsune Dec 11, 2024

Choose a reason for hiding this comment

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

Nit: Consider SCREAMING_SNAKE_CASE for immutables, similar to constants. There is no hard and fast rule for this but this is a common pattern.


/// @dev Make this configurable
uint256 internal immutable numPbhPerMonth = 30;

/// @dev Whether a nullifier hash has been used already. Used to guarantee an action is only performed once by a single person
mapping(uint256 => bool) internal nullifierHashes;

///////////////////////////////////////////////////////////////////////////////
/// Functions ///
//////////////////////////////////////////////////////////////////////////////

/// @param _worldId The WorldID instance that will verify the proofs
constructor(
IWorldIDGroups _worldId
) {
worldId = _worldId;
}

function verifyExternalNullifier(ExternalNullifier memory externalNullifer) public view {
require(externalNullifer.year == BokkyPooBahsDateTimeLibrary.getYear(block.timestamp), InvalidExternalNullifierYear());
require(externalNullifer.month == BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp), InvalidExternalNullifierMonth());
require(externalNullifer.pbhNonce <= numPbhPerMonth, InvalidPbhNonce());
}

/// @param userOp A packed user operation, used to generate the signal hash
/// @param root The root of the Merkle tree (returned by the JS widget).
/// @param nullifierHash The nullifier hash for this proof, preventing double signaling (returned by the JS widget).
/// @param proof The zero-knowledge proof that demonstrates the claimer is registered with World ID (returned by the JS widget).
function verifyPbhProof(
PackedUserOperation memory userOp,
uint256 root,
uint256 nullifierHash,
ExternalNullifier memory externalNullifier,

Choose a reason for hiding this comment

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

how are we getting the externalNullifier data here - do we need to add this with the proof in the signature

or is this determined by the builder?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@karankurbur Yes, this should be decoded from the signature on the UserOperation

uint256[8] memory proof
) external {
// First, we make sure this person hasn't done this before
if (nullifierHashes[nullifierHash]) revert InvalidNullifier();

// We now generate the signal hash from the sender, nonce, and calldata
uint256 signalHash = abi.encodePacked(
userOp.sender,
userOp.nonce,
userOp.callData
).hashToField();

// Verify the external nullifier
verifyExternalNullifier(externalNullifier);

uint256 externalNullifierHash = abi.encode(externalNullifier).hashToField();

// We now verify the provided proof is valid and the user is verified by World ID
worldId.verifyProof(
root,
groupId,
signalHash,
nullifierHash,
externalNullifierHash,
proof
);

// We now record the user has done this, so they can't do it again (proof of uniqueness)
nullifierHashes[nullifierHash] = true;

emit PBH(nullifierHash);
}
}

12 changes: 12 additions & 0 deletions pbh-verifier/src/helpers/ByteHasher.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.10;

library ByteHasher {
/// @dev Creates a keccak256 hash of a bytestring.
/// @param value The bytestring to hash
/// @return The hash of the specified value
/// @dev `>> 8` makes sure that the result is included in our field
function hashToField(bytes memory value) internal pure returns (uint256) {
return uint256(keccak256(abi.encodePacked(value))) >> 8;
}
}
16 changes: 16 additions & 0 deletions pbh-verifier/test/PBHVerifier.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {PBHVerifier} from "../src/PBHVerifier.sol";

contract PBHVerifierTest is Test {
PBHVerifier public pbhVerifier;

function setUp() public {
// pbhVerifier = new PBHVerifier();
}

function testTest() public {
}
}
Loading