Implement veNFT interface #23

hwrdtm · 2025-11-29T04:32:18Z

Closes NODE-4819.

github-actions · 2025-12-01T19:13:27Z

PASS [ 43.765s] (3/3) lit_node::test toxiproxy::perf_tests::load_with_no_latency
PASS [ 43.848s] (2/3) lit_node::test toxiproxy::perf_tests::load_with_50ms_latency_single_link
PASS [ 93.991s] (1/3) lit_node::test toxiproxy::perf_tests::load_with_50ms_latency_all_links

hwrdtm · 2025-12-01T21:51:10Z

Just tried running the forge invariant tests locally and no issues there. Might need to run few more times 🤔 forge test --match-test invariant -vv --no-match-test Skip

We need to reeserve the balanceOf(address) for StakingNFTFacet.

glitch003

overall, looks good. how does an existing delegated staker get an NFT? like what's the migration path?

glitch003 · 2025-12-11T05:52:43Z

blockchain/contracts/contracts/interfaces/IERC721.sol

@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: MIT
+// OpenZeppelin Contracts (last updated v4.9.0) (token/ERC721/IERC721.sol)


we should import this from openzeppelin instead of pasting it here, right?

On line 8 (few lines below) I explain why I am doing this approach instead. Open to better ideas tho!

glitch003 · 2025-12-11T07:01:36Z

blockchain/contracts/contracts/lit-node/Staking/StakingNFTFacet.sol

+                mappedStakeRecord.operatorStakerAddress
+            ) == 0
+        ) {
+            revert NoEmptyStakeRecordSlots();


under what conditions can this happen? the user has too many stake records? what's the limit?

Each staker vault holds only 30 stake records. Keep in mind a staker vault is specific to a validator, so a delegating staker can have more than 30 stake records if delegating against more than one validator.

glitch003 · 2025-12-11T07:03:01Z

blockchain/contracts/contracts/lit-node/Staking/StakingNFTFacet.sol

+
+        // Add NFT
+        LibStakingNFT._addTokenTo(to, tokenId);
+        // Set the block of ownership transfer (for Flash NFT protection)


what's flash nft protection?

Copied over from aerodrome contracts. It's basically to prevent buyers from doing additional actions in the same transaction / block as when they got transferred the NFT. For example, in aerodrome they prevent poking, delegating and voting in the same TX / block as when the transferred happened.

glitch003 · 2025-12-11T07:05:18Z

blockchain/contracts/contracts/lit-node/Staking/StakingNFTFacet.sol

+        address owner = ownerOf(tokenId);
+        bool spenderIsOwner = owner == spender;
+        bool spenderIsApproved = spender == s().tokenToApprovals[tokenId];
+        bool spenderIsApprovedForAll = (s().ownerToOperators[owner])[spender];


i don't understand spenderIsApprovedForAll, can you explain how that works? is it an admin function?

An owner can approve another address for permission over all of that owner's tokens.

glitch003 · 2025-12-11T07:24:50Z

also question - i didn't see the NFT name or metadata uri stuff in the erc721 interface. did i miss it? i wish we could inherit from openzeppelin erc721 but it doesn't use diamond storage.

hwrdtm · 2025-12-11T07:33:01Z

overall, looks good. how does an existing delegated staker get an NFT? like what's the migration path?

We would need to script / use admin powers to manipulate existing state to account for the existing stakes.

Copilot

Pull request overview

This PR implements a veNFT (voting-escrow NFT) interface for the staking system. The changes introduce ERC721 functionality to represent staking positions as transferable NFTs, allowing users to trade or transfer their staked positions while maintaining proper accounting.

Key changes:

Introduces veNFT interface with ERC721 standard functions (approve, transfer, balanceOf, etc.)
Renames the staking balance query function from balanceOf to selfStakeBalanceOf to avoid collision with ERC721's balanceOf
Adds NFT-specific tracking with tokenId fields in StakeRecord structs
Updates event handling in the listener to match new contract events

Reviewed changes

Copilot reviewed 29 out of 36 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
rust/lit-node/lit-node/src/peers/peer_state/listener.rs	Changes event filter from `RequestToJoinFilter` to `RequestToJoin1Filter` (appears to contain naming issue)
rust/lit-core/lit-blockchain/src/contracts/pkp_helper.rs	Updates contract bytecode for PKPHelper contract
rust/lit-core/lit-blockchain/abis/Staking.json	Extensive ABI updates: adds veNFT/ERC721 functions, renames `balanceOf` to `selfStakeBalanceOf`, adds tokenId fields to StakeRecord structs
blockchain/contracts/test/lit-node/Staking.js	Updates test calls from `balanceOf` to `selfStakeBalanceOf`
blockchain/contracts/test/lit-node/PubkeyRouter.js	Adds StakingNFTFacet to the deployment configuration

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

rust/lit-node/lit-node/src/peers/peer_state/listener.rs

hwrdtm · 2025-12-11T16:20:43Z

also question - i didn't see the NFT name or metadata uri stuff in the erc721 interface. did i miss it? i wish we could inherit from openzeppelin erc721 but it doesn't use diamond storage.

There actually are ERC721 implementations for diamond contracts that we are using already - check out PKPNFTFacet using ERC721Upgradeable. However, we need to implement our own custom implementation anyways, and for the same reason as Implement veNFT interface #23 (comment), we can't just inherit that.
I didn't implement it yet, I didn't think we needed it. Should be simple to implement, though. In any case, I think we could do in the next PR though.

hwrdtm self-assigned this Nov 29, 2025

hwrdtm marked this pull request as ready for review December 3, 2025 04:03

hwrdtm requested review from GTC6244 and glitch003 December 3, 2025 04:03

hwrdtm force-pushed the feature/node-4819-stakingdiamond-nftfacet branch from a7ce0ec to 7befe08 Compare December 3, 2025 04:48

hwrdtm changed the base branch from master to hwrdtm/remove-unused-staking-mapping December 3, 2025 04:51

hwrdtm force-pushed the feature/node-4819-stakingdiamond-nftfacet branch from 69aad84 to 852a7f5 Compare December 3, 2025 04:58

hwrdtm force-pushed the hwrdtm/remove-unused-staking-mapping branch from a4fae38 to f7c391c Compare December 3, 2025 18:49

hwrdtm force-pushed the feature/node-4819-stakingdiamond-nftfacet branch 2 times, most recently from e8f9f44 to dcd1142 Compare December 3, 2025 19:02

Base automatically changed from hwrdtm/remove-unused-staking-mapping to master December 3, 2025 22:54

hwrdtm added 17 commits December 3, 2025 14:57

Update ABIs and rust bindings

08b5a30

Implement veNFT interface and link with existing staking system

bdd2fdd

Rename stake balanceOf

5f600f5

We need to reeserve the balanceOf(address) for StakingNFTFacet.

Fix tests after renaming balanceOf

9d499dc

Fix wagmi build

9258345

wagmi generate

118905c

Add StakingNFTFacet to hardhat config

9cd6e7d

Update ABIs and rust bindings

1e6eec2

Deploy StakingNFTFacet

0d1919a

Fix tests

7d54264

Fix tests

3c4f12e

Implement some StakingNFTFacet tests

24d528d

Implement tests

ba81aee

Prettier

fd9004e

Update ABIs and rust bindings

db85fbd

Update ABIs and rust bindings

9c7b80e

Fix build

fc092f4

Update ABIs and rust bindings

3a9d9ec

hwrdtm force-pushed the feature/node-4819-stakingdiamond-nftfacet branch from c4c6deb to 3a9d9ec Compare December 3, 2025 22:57

Merge branch 'master' into feature/node-4819-stakingdiamond-nftfacet

a2a58db

glitch003 reviewed Dec 11, 2025

View reviewed changes

glitch003 requested a review from Copilot December 11, 2025 07:33

Copilot started reviewing on behalf of glitch003 December 11, 2025 07:33 View session

Copilot AI reviewed Dec 11, 2025

View reviewed changes

rust/lit-node/lit-node/src/peers/peer_state/listener.rs Show resolved Hide resolved

		@@ -0,0 +1,159 @@
		// SPDX-License-Identifier: MIT
		// OpenZeppelin Contracts (last updated v4.9.0) (token/ERC721/IERC721.sol)

Implement veNFT interface #23

Are you sure you want to change the base?

Implement veNFT interface #23

Uh oh!

Conversation

hwrdtm commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hwrdtm commented Dec 1, 2025

Uh oh!

glitch003 left a comment

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!

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!

glitch003 commented Dec 11, 2025

Uh oh!

hwrdtm commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

hwrdtm commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hwrdtm commented Nov 29, 2025 •

edited

Loading

github-actions bot commented Dec 1, 2025 •

edited

Loading

hwrdtm commented Dec 11, 2025 •

edited

Loading