feat: Add ECDSA validator check when permitting an app version by Spacesai1or · Pull Request #460 · LIT-Protocol/Vincent

Spacesai1or · 2026-01-14T22:31:02Z

Description

Inits new facet: packages/libs/contracts-sdk/contracts/facets/VincentZeroDevConfigFacet.sol with a getter/setter for setting the canonical ZeroDev ECDSA Validator address (source from https://github.com/zerodevapp/kernel?tab=readme-ov-file)
Updates Diamond constructor to require _ecdsaValidatorAddress param and handles registering newVincentZeroDevConfigFacet
Updates Makefile with new required ENV: VINCENT_ECDSA_VALIDATOR_ADDRESS and adds new cmd: set-ecdsa-validator to execute the set validator address script
Adds VincentZeroDevStorage to packages/libs/contracts-sdk/contracts/LibVincentDiamondStorage.sol to store ecdsaValidatorAddress
Updates packages/libs/contracts-sdk/contracts/facets/VincentUserFacet.sol:
- Adds two new function modifiers:
  - onlySmartAccountOwner which is used by permitAppVersion to check if msg.sender is the registered ECDSA signer for the agentAddress
  - onlyAgentOwner which is used by unPermitAppVersion, rePermitApp, and setAbilityPolicyParameters to check if msg.sender is address registeredOwner = us_.registeredAgentAddressToUserAddress[agentAddress];
In packages/libs/contracts-sdk/contracts/libs/LibVincentUserFacet.sol AgentNotRegisteredToUser was replaced with NotAgentOwner which is used by both above function modifiers
Init new test helper script: packages/libs/contracts-sdk/script/FetchECDSAValidatorBytecode.sol which is used to fetch the ZeroDev ECDSA Validator bytecode from Base deployment and write it to a file so the tests can write the bytecode to the expected Base/Base sepolia address
Init new script: packages/libs/contracts-sdk/script/SetEcdsaValidatorAddress.sol for setting the ZeroDev ECDSA Validator address used by the onlySmartAccountOwner modifier in VincentUserFacet
Updates: packages/libs/contracts-sdk/script/UpdateFacet.sol to handle new VincentZeroDevConfigFacet

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Existing tests were failing with NotAgentOwner, after updating them to use the test helper which sets the owner of the smart account to the test address, tests are passing
Added new test script for new facet

Checklist:

I created a release plan (nx release plan) describing my changes and the version bump
My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

vercel · 2026-01-14T22:31:11Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
vincent-app-dashboard	Ready	Preview, Comment	Jan 29, 2026 8:39pm

Copilot

Pull request overview

This PR adds ECDSA validator integration to verify smart account ownership through ZeroDev's ECDSA validator contract when users permit app versions. The PR introduces a new access control mechanism that checks the smart account owner via an external validator contract before allowing operations.

Changes:

Adds VincentZeroDevConfigFacet for managing the ZeroDev ECDSA validator address configuration
Implements onlySmartAccountOwner modifier in VincentUserFacet to verify smart account ownership via the ECDSA validator for permitAppVersion
Updates onlyAgentOwner modifier to check internal state for other user functions (unPermitAppVersion, rePermitApp, setAbilityPolicyParameters)

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`contracts/facets/VincentZeroDevConfigFacet.sol`	New facet providing getter/setter for ECDSA validator address
`contracts/facets/VincentUserFacet.sol`	Adds two new modifiers for ownership verification and applies them to user functions
`contracts/libs/LibVincentUserFacet.sol`	Replaces `AgentNotRegisteredToUser` error with more descriptive `NotAgentOwner` error
`contracts/VincentDiamond.sol`	Updates constructor to accept and store ECDSA validator address, registers new facet
`contracts/LibVincentDiamondStorage.sol`	Adds `VincentZeroDevStorage` library for storing ECDSA validator address
`test/TestHelpers.sol`	New helper contract for setting up ECDSA validator in tests using bytecode fixtures
`test/fixtures/ECDSAValidatorBytecode.txt`	Contains ECDSA validator bytecode fetched from Base mainnet
`script/FetchECDSAValidatorBytecode.sol`	Helper script to fetch and save ECDSA validator bytecode from Base
`script/SetEcdsaValidatorAddress.sol`	Script for updating ECDSA validator address on deployed contracts
`script/UpdateFacet.sol`	Adds support for updating `VincentZeroDevConfigFacet`
`script/DeployVincentDiamond.sol`	Updates deployment to require ECDSA validator address parameter
`Makefile`	Adds `VINCENT_ECDSA_VALIDATOR_ADDRESS` env var requirement and `set-ecdsa-validator` command
`foundry.toml`	Adds filesystem permissions for test fixtures directory
`abis/VincentZeroDevConfigFacet.abi.json`	ABI for new config facet
`abis/VincentUserFacet.abi.json`	Updates ABI with new error signature
`test/facets/*.t.sol`	Updates tests to register smart account owners and fix expected error types

Comments suppressed due to low confidence (1)

packages/libs/contracts-sdk/contracts/facets/VincentUserFacet.sol:127

The AgentRegisteredToDifferentUser check at lines 123-127 is now redundant since the onlySmartAccountOwner modifier already validates that the caller is the owner according to the ECDSA validator. If the ECDSA validator check passes, this internal state check should always pass as well (assuming state consistency). Consider removing this redundant check or adding a comment explaining why both checks are necessary.

        // Check if the agent is already registered to a different user
        address registeredUserAddress = us_.registeredAgentAddressToUserAddress[agentAddress];
        if (registeredUserAddress != address(0) && registeredUserAddress != sender) {
            revert LibVincentUserFacet.AgentRegisteredToDifferentUser(registeredUserAddress);
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DashKash54

Looks good but left some design questions

DashKash54 · 2026-01-21T17:17:09Z

+    }
+
+    /**
+     * @notice Validates that the caller is the registered owner of the agent in Vincent's internal state


Isn't the user's EOA the registered owner Agent smart account in registeredAgentAddressToUserAddress, can it ever be separate? Why have two separate modifiers onlyAgentOwner and onlySmartAccountOwner?

it looks like onlySmartAccountOwner is only used on installation. we use it to set the owner, and then onlyAgentOwner checks that owner.

But why have two if they're doing the same thing?

One of the modifiers calls the ecdsa validator storage contract which is comparatively expensive, but required when the user permits the app for the smart account for the first time, then because we store the owner in the diamond storage, the second modifier can just read from storage which saves gas

DashKash54 · 2026-01-21T17:23:25Z

+/**
+ * @title FetchECDSAValidatorBytecode
+ * @notice Script to fetch the ECDSA validator bytecode from Base Mainnet and save it
+ * @dev Run with: forge script script/FetchECDSAValidatorBytecode.sol:FetchECDSAValidatorBytecode --rpc-url https://mainnet.base.org


This is used only in our local tests but not used during the deployment or setup of ecdsaValidator?

Yes, this allows us to test against the actual ecdsa validator that's deployed on Base. The tests will etch the bytecode into the expected address to simulate deployment on Base mainnet/sepolia

DashKash54 · 2026-01-21T17:29:03Z

+     *      The ECDSA validator has: mapping(address => address) public ecdsaValidatorStorage;
+     *      This mapping is at slot 0, so we calculate: keccak256(abi.encode(smartAccount, 0))
+     */
+    function registerSmartAccountOwner(address smartAccount, address owner) internal {


How will we set this up for the Agent smart account IRL?

the onlySmartAccountOwner takes care of this when permitting the app. like onlySmartAccountOwner validates the address matches the smart account owner. and then when permitting the app we store this owner. is this correct @Spacesai1or ?

Yes, the EOA is set as the validator when the smart account is deployed, then we record the verified owner EOA in the diamond storage when an App is permitted

DashKash54 · 2026-01-21T17:29:51Z

+
+        // Register smart account owners
+        // This simulates Frank and George owning their agents (smart accounts)
+        registerSmartAccountOwner(APP_USER_FRANK, USER_FRANK);


Any tests to verify that permitApp doesn't work if the signer isn't ECDSA?

hmm can you clarify this question @DashKash54 ? why would the signer be anything else, and yes, if it was something else (not ecdsa) then it wouldn't work?

Just to validate that it won't work if the signer is anything else, confirming unhappy path doesn't work

DashKash54

Approving but we have 2 unresolved comments:

Why have two separate modifier: #460 (comment)
How does the ownerSmartAccount owner work for agent: #460 (comment)

feat: Add ECDSA validator check when permitting an app version

c7cc9ea

vercel Bot deployed to Preview January 14, 2026 22:32 View deployment

chore: forge fmt

3e17f78

Spacesai1or marked this pull request as ready for review January 14, 2026 22:46

Spacesai1or requested review from Ansonhkg, DashKash54 and awisniew207 as code owners January 14, 2026 22:46

Copilot AI review requested due to automatic review settings January 14, 2026 22:46

Copilot started reviewing on behalf of Spacesai1or January 14, 2026 22:47 View session

vercel Bot deployed to Preview January 14, 2026 22:49 View deployment

Copilot AI reviewed Jan 14, 2026

View reviewed changes

DashKash54 requested changes Jan 21, 2026

View reviewed changes

fix: bump solidity version on validator set script

ffc6931

vercel Bot deployed to Preview January 29, 2026 20:39 View deployment

DashKash54 self-requested a review January 30, 2026 00:13

DashKash54 approved these changes Jan 30, 2026

View reviewed changes

Spacesai1or merged commit 1fa0a48 into main Jan 31, 2026
5 checks passed

Spacesai1or deleted the feat/ecdsa-validator-contracts branch January 31, 2026 07:58

Conversation

Spacesai1or commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

vercel Bot commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DashKash54 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DashKash54 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Spacesai1or commented Jan 14, 2026 •

edited

Loading

vercel Bot commented Jan 14, 2026 •

edited

Loading

DashKash54 left a comment •

edited

Loading