Skip to content

💥 introduced register-unregister wallet in identity#138

Open
mohamadhammoud wants to merge 19 commits intodevelopfrom
feature/BT-1447-global-identity-registry-id-factory
Open

💥 introduced register-unregister wallet in identity#138
mohamadhammoud wants to merge 19 commits intodevelopfrom
feature/BT-1447-global-identity-registry-id-factory

Conversation

@mohamadhammoud
Copy link
Collaborator

No description provided.

@mohamadhammoud mohamadhammoud self-assigned this Jan 3, 2026
Copy link
Collaborator

@pgonday pgonday left a comment

Choose a reason for hiding this comment

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

Maybe for better consistency with existing functions (linkWallet/unlinkWallet), rename

  • registerWalletToIdentity -> linkWalletWithSignature
  • unregisterWalletFromIdentity -> unlinkWalletWithSignatureg

Also the signature is not protected against replay, for example the same signature can be used again after unlinking, but maybe we can avoid this with a short validity.

// token linked to an ONCHAINID
mapping(address => address) private _tokenAddress;

// nonce per wallet for signature replay protection
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inherits OpenZeppelin Nonces instead of custom implementation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/**
* @dev See {IIdFactory-walletNonce}.
*/
function walletNonce(
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove (OZ Nonces inherits)

Copy link
Collaborator

@pgonday pgonday left a comment

Choose a reason for hiding this comment

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

EIP-712 is not sufficient to avoid replay attack.
Currently it is possible to call the same signature:

  • link (Creates an EIP-712 signature with a 1-hour expiry)
  • unlink
  • Replay: Replays the exact same still-valid signature to re-link the wallet -> should revert, which is not the case
    Add and use OpenZeppelin Nonces

…id-factory

# Conflicts:
#	.github/scripts/coverage.sh
#	.github/workflows/foundry.yml
#	.husky/commit-msg
#	.husky/post-checkout
#	.husky/post-merge
#	.husky/pre-commit
#	contracts/ClaimIssuer.sol
#	contracts/Identity.sol
#	contracts/IdentityUtilities.sol
#	contracts/KeyManager.sol
#	contracts/factory/IIdFactory.sol
#	contracts/factory/IdFactory.sol
#	contracts/gateway/Gateway.sol
#	contracts/interface/IClaimIssuer.sol
#	contracts/interface/IERC734.sol
#	contracts/interface/IERC735.sol
#	contracts/interface/IIdentity.sol
#	contracts/interface/IIdentityUtilities.sol
#	contracts/proxy/ClaimIssuerProxy.sol
#	contracts/proxy/IdentityUtilitiesProxy.sol
#	contracts/proxy/ImplementationAuthority.sol
#	foundry.toml
#	remappings.txt
#	soldeer.lock
#	test/Proxy.t.sol
#	test/claim-issuers/ClaimIssuer.t.sol
#	test/claim-issuers/ClaimTo.t.sol
#	test/factory/ClaimIssuerFactory.t.sol
#	test/factory/IdFactory.t.sol
#	test/factory/TokenOid.t.sol
#	test/gateway/Gateway.t.sol
#	test/helpers/ClaimIssuerHelper.sol
#	test/helpers/ClaimSignerHelper.sol
#	test/helpers/IdentityHelper.sol
#	test/helpers/OnchainIDSetup.sol
#	test/identities/Claims.t.sol
#	test/identities/Executions.t.sol
#	test/identities/Init.t.sol
#	test/identities/Keys.t.sol
#	test/identities/ProxyPattern.t.sol
#	test/identities/VersionUpgrade.t.sol
#	test/identity-utilities/IdentityUtilities.t.sol
#	test/mocks/RevertingIdentity.sol
#	test/mocks/TestIdentityUtilities.sol
#	test/utils/Constants.sol
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

Changes to gas cost

Generated at commit: 4a1ece4c89ec37fa975c71e34231111ef189bb36, compared to commit: ac0c249219b058823d039cec0f6671dabcb0126e

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
IdentityUtilities upgradeToAndCall -19,405 ✅ -75.88%
IdFactory getIdentity
linkWallet
unlinkWallet
+2,217 ❌
+25,107 ❌
+5,680 ❌
+51.74%
+33.25%
+19.38%
ERC1967Proxy upgradeToAndCall -29,150 ✅ -49.14%
Identity version -105 ✅ -17.16%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
IdentityUtilities 2,501,374 (-145,906) addTopic
getClaimsWithTopicInfo
getFieldNames
getFieldTypes
getSchema
getTopic
getTopicInfos
initialize
removeTopic
updateTopic
upgradeToAndCall
3,588 (-15)
92,060 (-2,271)
2,918 (+3)
2,949 (+3)
3,216 (+3)
8,957 (-120)
854 (0)
74,076 (+368)
2,790 (-2,317)
3,611 (-2,306)
3,565 (-7,780)
-0.42%
-2.41%
+0.10%
+0.10%
+0.09%
-1.32%
0.00%
+0.50%
-45.37%
-38.97%
-68.58%
325,468 (-780)
140,800 (-4,253)
22,933 (-460)
22,955 (-461)
34,888 (-595)
31,186 (-811)
42,654 (-1,049)
74,076 (+368)
55,740 (-3,205)
91,750 (-2,442)
6,168 (-19,405)
-0.24%
-2.93%
-1.97%
-1.97%
-1.68%
-2.53%
-2.40%
+0.50%
-5.44%
-2.59%
-75.88%
302,900 (-169)
140,800 (-4,253)
15,612 (-201)
15,632 (-201)
30,656 (-405)
24,411 (-632)
36,054 (-890)
74,076 (+368)
70,534 (+35)
47,287 (-169)
3,565 (-29,123)
-0.06%
-2.93%
-1.27%
-1.27%
-1.30%
-2.52%
-2.41%
+0.50%
+0.05%
-0.36%
-89.09%
1,527,059 (-2,005)
189,541 (-6,234)
80,827 (-2,064)
80,847 (-2,064)
59,639 (-1,233)
82,718 (-2,360)
93,184 (-2,484)
74,076 (+368)
70,534 (+35)
211,296 (-373)
11,374 (-21,314)
-0.13%
-3.18%
-2.49%
-2.49%
-2.03%
-2.77%
-2.60%
+0.50%
+0.05%
-0.18%
-65.20%
55 (0)
2 (0)
9 (0)
9 (0)
11 (0)
10 (0)
6 (0)
61 (0)
9 (0)
13 (0)
3 (0)
IdFactory 2,286,340 (+563,645) addTokenFactory
createIdentity
createIdentityWithManagementKeys
createTokenIdentity
getIdentity
getToken
getWallets
isSaltTaken
isTokenFactory
linkWallet
unlinkWallet
23,753 (0)
24,656 (0)
25,946 (+22)
27,309 (0)
2,830 (0)
2,620 (-22)
5,331 (+146)
2,857 (+53)
2,658 (+89)
21,612 (0)
21,613 (-22)
0.00%
0.00%
+0.08%
0.00%
0.00%
-0.83%
+2.82%
+1.89%
+3.46%
0.00%
-0.10%
37,727 (-6)
481,283 (+19,040)
198,556 (-17,544)
433,796 (+116)
6,502 (+2,217)
2,620 (-22)
6,459 (-244)
2,857 (+53)
2,658 (+89)
100,616 (+25,107)
34,993 (+5,680)
-0.02%
+4.12%
-8.12%
+0.03%
+51.74%
-0.83%
-3.64%
+1.89%
+3.46%
+33.25%
+19.38%
47,524 (-12)
491,598 (+19,087)
31,579 (+131)
449,685 (-3,434)
7,215 (+2,384)
2,620 (-22)
6,459 (-1,003)
2,857 (+53)
2,658 (+89)
103,040 (+25,163)
37,123 (+10,831)
-0.03%
+4.04%
+0.42%
-0.76%
+49.35%
-0.83%
-13.44%
+1.89%
+3.46%
+32.31%
+41.20%
47,524 (-12)
491,661 (+19,090)
546,750 (-3,338)
449,685 (-3,434)
7,220 (+2,389)
2,620 (-22)
7,587 (+125)
2,857 (+53)
2,658 (+89)
103,040 (+25,163)
47,285 (+6,793)
-0.03%
+4.04%
-0.61%
-0.76%
+49.45%
-0.83%
+1.68%
+1.89%
+3.46%
+32.31%
+16.78%
7 (0)
308 (+55)
9 (+1)
159 (+28)
38 (+27)
2 (0)
6 (+3)
3 (0)
3 (0)
222 (+113)
14 (+9)
ERC1967Proxy 356,504 (-1,564) fallback
upgradeToAndCall
7,145 (-105)
30,165 (-29,150)
-1.45%
-49.14%
91,129 (-387)
30,165 (-29,150)
-0.42%
-49.14%
18,138 (-311)
30,165 (-29,150)
-1.69%
-49.14%
1,195,152 (-3,414)
30,165 (-29,150)
-0.28%
-49.14%
343 (+49)
1 (0)
ClaimIssuer 3,150,069 (-197,239) addClaimTo
addKey
execute
initialize
isClaimRevoked
isClaimValid
proxiableUUID
revokeClaim
revokeClaimBySignature
6,447 (0)
126,934 (-4)
833,130 (-2,322)
227,407 (-85)
3,224 (-206)
2,222 (-105)
393 (+69)
5,710 (-22)
5,842 (+45)
0.00%
-0.00%
-0.28%
-0.04%
-6.01%
-4.51%
+21.30%
-0.38%
+0.78%
292,173 (-1,628)
127,077 (-35)
998,549 (-2,868)
227,407 (-85)
3,224 (-206)
12,910 (-255)
393 (+69)
46,414 (-1,056)
20,106 (+44)
-0.55%
-0.03%
-0.29%
-0.04%
-6.01%
-1.94%
+21.30%
-2.22%
+0.22%
215,756 (-1,845)
126,934 (-4)
998,549 (-2,868)
227,407 (-85)
3,224 (-206)
13,203 (-311)
393 (+69)
56,354 (-1,331)
21,262 (+43)
-0.85%
-0.00%
-0.29%
-0.04%
-6.01%
-2.30%
+21.30%
-2.31%
+0.20%
735,482 (-3,405)
148,790 (-4)
1,163,968 (-3,414)
227,407 (-85)
3,224 (-206)
13,240 (-439)
393 (+69)
67,241 (-1,537)
32,059 (+43)
-0.46%
-0.00%
-0.29%
-0.04%
-6.01%
-3.21%
+21.30%
-2.23%
+0.13%
8 (0)
152 (+27)
2 (0)
156 (+27)
2 (0)
178 (+27)
1 (0)
4 (0)
4 (0)
Identity 2,415,475 (-37,115) addClaim
addKey
approve
execute
getClaim
getExecutionData
initialize
isClaimValid
multicall
version
6,998 (0)
188 (0)
5,414 (0)
76,759 (-41)
15,527 (-123)
10,286 (-41)
2,838 (-55)
2,061 (-105)
465,860 (-2,036)
507 (-105)
0.00%
0.00%
0.00%
-0.05%
-0.79%
-0.40%
-1.90%
-4.85%
-0.44%
-17.16%
298,110 (+520)
182,822 (+1,049)
176,537 (-351)
270,580 (+11,261)
21,230 (-367)
17,055 (-252)
225,722 (+419)
7,884 (-89)
466,601 (-3,085)
507 (-105)
+0.17%
+0.58%
-0.20%
+4.34%
-1.70%
-1.46%
+0.19%
-1.12%
-0.66%
-17.16%
306,900 (-1,092)
188,164 (0)
121,467 (0)
258,180 (+86,056)
22,443 (-443)
14,734 (-201)
227,390 (+71)
7,633 (-105)
466,601 (-3,085)
507 (-105)
-0.35%
0.00%
0.00%
+50.00%
-1.94%
-1.35%
+0.03%
-1.36%
-0.66%
-17.16%
371,784 (-1,476)
188,164 (0)
383,334 (-1,092)
700,590 (-2,093)
28,822 (-571)
42,756 (-1,001)
227,390 (+71)
12,944 (-105)
467,343 (-4,134)
507 (-105)
-0.40%
0.00%
-0.28%
-0.30%
-1.94%
-2.29%
+0.03%
-0.80%
-0.88%
-17.16%
175 (+27)
329 (+54)
15 (0)
57 (+28)
16 (0)
10 (0)
475 (+82)
8 (0)
2 (0)
5 (0)
IdentityProxy 390,981 (-1,321) fallback 2,497 (0) 0.00% 217,253 (+10,590) +5.12% 217,976 (0) 0.00% 730,352 (-2,093) -0.29% 659 (+109)
Gateway 1,306,749 (-7,793) callFactory
deployIdentityForWallet
deployIdentityWithSalt
deployIdentityWithSaltAndManagementKeys
24,459 (0)
21,632 (0)
24,840 (+1,228)
25,510 (+1,426)
0.00%
0.00%
+5.20%
+5.92%
35,971 (-129)
316,242 (+11,019)
165,645 (+5,886)
202,011 (+5,831)
-0.36%
+3.61%
+3.68%
+2.97%
29,892 (-188)
505,824 (+18,601)
31,496 (-105)
33,931 (-105)
-0.63%
+3.82%
-0.33%
-0.31%
53,563 (-200)
505,824 (+18,601)
506,720 (+18,880)
544,668 (+15,122)
-0.37%
+3.82%
+3.87%
+2.86%
3 (0)
5 (0)
7 (0)
6 (0)
ClaimIssuerFactory 706,816 (-19,477) deployClaimIssuer
deployClaimIssuerOnBehalf
23,580 (0)
23,775 (0)
0.00%
0.00%
214,600 (-1,563)
151,654 (-1,042)
-0.72%
-0.68%
215,153 (-1,563)
23,980 (0)
-0.72%
0.00%
404,515 (-3,126)
407,209 (-3,126)
-0.77%
-0.76%
4 (0)
3 (0)
IdentityUtilitiesProxy 203,465 (-1,111) fallback 5,744 (0) 0.00% 187,544 (-1,353) -0.72% 77,305 (+28) +0.04% 1,565,113 (-2,005) -0.13% 126 (0)
ImplementationAuthority 248,212 (-12) implementation 307 (0) 0.00% 1,675 (+6) +0.36% 2,307 (0) 0.00% 2,307 (0) 0.00% 1,637 (+273)
ClaimIssuerProxy 356,796 (-1,552)

@mohamadhammoud mohamadhammoud requested a review from pgonday March 6, 2026 12:22
@mohamadhammoud mohamadhammoud requested a review from pgonday March 6, 2026 13:37
@mohamadhammoud mohamadhammoud force-pushed the feature/BT-1447-global-identity-registry-id-factory branch from dd75ad8 to b88aff1 Compare March 9, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants