Skip to content

Feature/bt 1496 identity setup single transaction#142

Merged
mohamadhammoud merged 12 commits intodevelopfrom
feature/BT-1496-identity-setup-single-transaction
Apr 8, 2026
Merged

Feature/bt 1496 identity setup single transaction#142
mohamadhammoud merged 12 commits intodevelopfrom
feature/BT-1496-identity-setup-single-transaction

Conversation

@mohamadhammoud
Copy link
Copy Markdown
Collaborator

No description provided.

@mohamadhammoud mohamadhammoud self-assigned this Feb 19, 2026
@mohamadhammoud mohamadhammoud changed the base branch from main to develop February 19, 2026 03:19
Comment thread contracts/factory/ClaimIssuerFactory.sol Outdated
Comment thread contracts/factory/ClaimIssuerFactory.sol Outdated
Comment thread contracts/factory/IdFactory.sol Outdated
Comment thread contracts/Identity.sol
Comment thread contracts/factory/IdFactory.sol Outdated
Comment thread contracts/factory/IdFactory.sol Outdated
Comment thread contracts/ClaimIssuer.sol Outdated
Comment thread contracts/factory/IdFactory.sol Outdated
@mohamadhammoud mohamadhammoud marked this pull request as ready for review February 20, 2026 14:16
Copy link
Copy Markdown
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.

  • removeClaim can be called by CLAIM_ADDER with the execute function
  • no check for valid identityType

Comment thread contracts/KeyManager.sol Outdated
Comment thread contracts/factory/IdFactory.sol Outdated
}

// bootstraps an identity: adds claim adder keys, transfers ownership, removes factory key
function _setupIdentityKeys(
Copy link
Copy Markdown
Contributor

@Joachim-Lebrun Joachim-Lebrun Feb 24, 2026

Choose a reason for hiding this comment

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

to avoid code duplicates i would use this internal function in both identity deployment functions (1 or many management keys). instead of address _owner just put an array of management keys. for the simple deploy function that array only contains the owner address.

Comment thread contracts/libraries/IdentityTypes.sol
Comment thread contracts/ClaimIssuer.sol
import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract ClaimIssuer is IClaimIssuer, Identity, UUPSUpgradeable {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing natspec

Comment thread contracts/ClaimIssuer.sol
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract ClaimIssuer is IClaimIssuer, Identity, UUPSUpgradeable {
mapping(bytes => bool) public revokedClaims;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing natspec

Comment thread contracts/ClaimIssuer.sol Outdated
function initialize(
address initialManagementKey
address initialManagementKey,
uint256
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

linter is not happy here, need to fix it

Comment thread contracts/ClaimIssuer.sol Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fix natspec

Comment thread contracts/ClaimIssuer.sol Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 7, 2026

Changes to gas cost

Generated at commit: 2b9aa64920307427050331d03afac1efae67a9d4, compared to commit: ac0c249219b058823d039cec0f6671dabcb0126e

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
Identity removeKey -15,654 ✅ -63.42%
Gateway deployIdentityWithSalt +86,820 ❌ +54.34%
IdFactory createIdentityWithManagementKeys
createTokenIdentity
+44,779 ❌
+66,142 ❌
+20.72%
+15.25%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
Identity 2,515,949 (+63,359) addClaim
addKey
approve
execute
getClaim
getClaimIdsByTopic
getExecutionData
getKeyPurposes
getKeysByPurpose
initialize
isClaimValid
keyHasPurpose
multicall
removeClaim
removeKey
version
7,732 (+734)
188 (0)
5,392 (-22)
76,887 (+87)
15,628 (-22)
1,629 (+88)
10,416 (+89)
5,665 (+11)
5,544 (-22)
2,935 (+42)
2,166 (0)
2,947 (0)
467,830 (-66)
5,528 (-22)
5,583 (-22)
590 (-22)
+10.49%
0.00%
-0.41%
+0.11%
-0.14%
+5.71%
+0.86%
+0.19%
-0.40%
+1.45%
0.00%
0.00%
-0.01%
-0.40%
-0.39%
-3.59%
298,427 (+837)
169,608 (-12,165)
176,869 (-19)
269,990 (+10,671)
21,718 (+121)
5,010 (+129)
17,173 (-134)
6,411 (+11)
5,544 (-22)
247,915 (+22,612)
8,537 (+564)
4,619 (+214)
472,810 (+3,124)
49,091 (-6,512)
9,031 (-15,654)
590 (-22)
+0.28%
-6.69%
-0.01%
+4.12%
+0.56%
+2.64%
-0.77%
+0.17%
-0.40%
+10.04%
+7.07%
+4.86%
+0.67%
-11.71%
-63.42%
-3.59%
308,007 (+15)
160,264 (-27,900)
121,423 (-44)
175,562 (+3,438)
22,864 (-22)
5,629 (+88)
14,942 (+7)
5,665 (+11)
5,544 (-22)
249,796 (+22,477)
8,428 (+690)
5,256 (0)
472,810 (+3,124)
69,766 (-4,691)
8,619 (-1,877)
590 (-22)
+0.00%
-14.83%
-0.04%
+2.00%
-0.10%
+1.59%
+0.05%
+0.19%
-0.40%
+9.89%
+8.92%
0.00%
+0.67%
-6.30%
-17.88%
-3.59%
373,276 (+16)
188,164 (0)
384,419 (-7)
702,825 (+142)
29,371 (-22)
10,105 (+88)
43,846 (+89)
7,903 (+11)
5,544 (-22)
249,796 (+22,477)
13,049 (0)
7,568 (0)
477,791 (+6,314)
89,586 (-22)
56,636 (-22)
590 (-22)
+0.00%
0.00%
-0.00%
+0.02%
-0.07%
+0.88%
+0.20%
+0.14%
-0.40%
+9.89%
0.00%
0.00%
+1.34%
-0.02%
-0.04%
-3.59%
161 (+13)
723 (+448)
15 (0)
31 (+2)
18 (+2)
16 (+1)
11 (+1)
3 (0)
1 (0)
433 (+40)
9 (+1)
29 (+10)
2 (0)
7 (+1)
428 (+417)
5 (0)
Gateway 1,415,740 (+101,198) approveSignature
approvedSigners
callFactory
deployIdentityForWallet
deployIdentityWithSalt
deployIdentityWithSaltAndManagementKeys
revokeSignature
revokeSigner
transferFactoryOwnership
25,244 (-10)
2,600 (+55)
24,495 (+36)
22,315 (+683)
24,397 (+785)
24,946 (+862)
25,266 (+79)
23,792 (-22)
23,995 (-22)
-0.04%
+2.16%
+0.15%
+3.16%
+3.32%
+3.58%
+0.31%
-0.09%
-0.09%
26,722 (-10)
2,600 (+55)
36,181 (+81)
341,470 (+36,247)
246,579 (+86,820)
203,400 (+7,220)
41,616 (+81)
24,940 (-22)
29,010 (-11)
-0.04%
+2.16%
+0.22%
+11.88%
+54.34%
+3.68%
+0.20%
-0.09%
-0.04%
27,214 (-10)
2,600 (+55)
30,183 (+103)
546,963 (+59,740)
33,801 (+2,200)
35,124 (+1,088)
49,172 (+79)
24,835 (-22)
29,010 (-11)
-0.04%
+2.16%
+0.34%
+12.26%
+6.96%
+3.20%
+0.16%
-0.09%
-0.04%
27,709 (-10)
2,600 (+55)
53,866 (+103)
546,963 (+59,740)
730,094 (+242,254)
549,264 (+19,718)
49,184 (+91)
26,298 (-22)
34,026 (0)
-0.04%
+2.16%
+0.19%
+12.26%
+49.66%
+3.72%
+0.19%
-0.08%
0.00%
3 (0)
2 (0)
3 (0)
5 (0)
8 (+1)
6 (0)
6 (0)
4 (0)
2 (0)
ImplementationAuthority 248,224 (0) implementation 307 (0) 0.00% 1,184 (-485) -29.06% 307 (-2,000) -86.69% 2,307 (0) 0.00% 2,342 (+978)
IdFactory 1,892,077 (+169,382) addTokenFactory
createIdentity
createIdentityWithManagementKeys
createTokenIdentity
getIdentity
isSaltTaken
removeTokenFactory
transferOwnership
23,820 (+67)
25,521 (+865)
26,770 (+846)
27,994 (+685)
2,830 (0)
2,826 (+22)
23,798 (-22)
28,649 (+22)
+0.28%
+3.51%
+3.26%
+2.51%
0.00%
+0.78%
-0.09%
+0.08%
37,800 (+67)
525,394 (+63,151)
260,879 (+44,779)
499,822 (+66,142)
4,330 (+45)
2,826 (+22)
24,946 (-22)
28,649 (+22)
+0.18%
+13.66%
+20.72%
+15.25%
+1.05%
+0.78%
-0.09%
+0.08%
47,603 (+67)
534,803 (+62,292)
32,636 (+1,188)
519,146 (+66,027)
4,831 (0)
2,826 (+22)
24,841 (-22)
28,649 (+22)
+0.14%
+13.18%
+3.78%
+14.57%
0.00%
+0.78%
-0.09%
+0.08%
47,603 (+67)
876,148 (+403,577)
714,954 (+164,866)
693,993 (+240,874)
4,831 (0)
2,826 (+22)
26,304 (-22)
28,649 (+22)
+0.14%
+85.40%
+29.97%
+53.16%
0.00%
+0.78%
-0.08%
+0.08%
7 (0)
279 (+26)
9 (+1)
143 (+12)
12 (+1)
3 (0)
4 (0)
18 (+1)
ClaimIssuer 3,423,273 (+75,965) addClaimTo
addKey
execute
initialize
isClaimRevoked
isClaimValid
proxiableUUID
revokeClaim
revokeClaimBySignature
upgradeTo
6,492 (+45)
126,961 (+23)
836,270 (+818)
249,931 (+22,439)
3,386 (-44)
2,327 (0)
369 (+45)
5,710 (-22)
5,842 (+45)
3,448 (-44)
+0.70%
+0.02%
+0.10%
+9.86%
-1.28%
0.00%
+13.89%
-0.38%
+0.78%
-1.26%
294,054 (+253)
127,567 (+455)
1,001,850 (+433)
249,931 (+22,439)
3,386 (-44)
13,193 (+28)
369 (+45)
47,431 (-39)
20,107 (+45)
9,610 (-21)
+0.09%
+0.36%
+0.04%
+9.86%
-1.28%
+0.21%
+13.89%
-0.08%
+0.22%
-0.22%
218,046 (+445)
126,961 (+23)
1,001,850 (+433)
249,931 (+22,439)
3,386 (-44)
13,514 (0)
369 (+45)
57,641 (-44)
21,264 (+45)
9,610 (-21)
+0.20%
+0.02%
+0.04%
+9.86%
-1.28%
0.00%
+13.89%
-0.08%
+0.21%
-0.22%
738,964 (+77)
188,191 (+39,397)
1,167,431 (+49)
249,931 (+22,439)
3,386 (-44)
13,679 (0)
369 (+45)
68,734 (-44)
32,061 (+45)
15,772 (+1)
+0.01%
+26.48%
+0.00%
+9.86%
-1.28%
0.00%
+13.89%
-0.06%
+0.14%
+0.01%
8 (0)
137 (+12)
2 (0)
140 (+11)
2 (0)
165 (+14)
1 (0)
4 (0)
4 (0)
2 (0)
ClaimIssuerFactory 719,150 (-7,143) deployClaimIssuer
deployClaimIssuerOnBehalf
23,580 (0)
23,775 (0)
0.00%
0.00%
227,524 (+11,361)
160,270 (+7,574)
+5.26%
+4.96%
228,077 (+11,361)
23,980 (0)
+5.24%
0.00%
430,363 (+22,722)
433,057 (+22,722)
+5.57%
+5.54%
4 (0)
3 (0)
IdentityProxy 415,327 (+23,025) fallback 2,585 (+88) +3.52% 205,038 (-1,625) -0.79% 217,976 (0) 0.00% 736,074 (+3,629) +0.50% 609 (+59)
ERC1967Proxy 380,795 (+22,727) fallback 7,250 (0) 0.00% 90,860 (-656) -0.72% 18,449 (0) 0.00% 1,198,615 (+49) +0.00% 320 (+26)
IdentityUtilities 2,647,280 (0) getClaimsWithTopicInfo 94,485 (+154) +0.16% 145,284 (+231) +0.16% 145,284 (+231) +0.16% 196,083 (+308) +0.16% 2 (0)
ClaimIssuerProxy 381,075 (+22,727) fallback 10,121 (0) 0.00% 27,323 (-15) -0.05% 29,765 (-44) -0.15% 42,085 (+1) +0.00% 3 (0)
IdentityUtilitiesProxy 204,576 (0) fallback 5,744 (0) 0.00% 188,901 (+4) +0.00% 77,277 (0) 0.00% 1,567,118 (0) 0.00% 126 (0)

Comment thread contracts/libraries/IdentityTypes.sol
@mohamadhammoud mohamadhammoud force-pushed the feature/BT-1496-identity-setup-single-transaction branch from 24a5494 to 806a427 Compare March 12, 2026 12:35
@mohamadhammoud mohamadhammoud force-pushed the feature/BT-1496-identity-setup-single-transaction branch from 806a427 to 5658e3d Compare March 12, 2026 12:41
@mohamadhammoud mohamadhammoud force-pushed the feature/BT-1496-identity-setup-single-transaction branch from 5658e3d to da5eb09 Compare March 12, 2026 12:43
@Joachim-Lebrun
Copy link
Copy Markdown
Contributor

please add a function on the gateway that allows the gateway owner to add a token factory address on the IdFactory. The function already exists on the factory but it is not exposed on the gateway

Comment thread contracts/ClaimIssuer.sol Outdated
override
initializer
{
__ClaimIssuer_init(initialManagementKey);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can't you just do super.initialize(initialManagementKey, IdentityTypes.CLAIM_ISSUER);
and remove __ClaimIssuer_init

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

c16f928
We can't use super.initialize() directly because both functions are external, I removed __ClaimIssuer_init entirely and inlined its logic directly into initialize

@mohamadhammoud mohamadhammoud requested a review from pgonday March 19, 2026 10:38
@mohamadhammoud mohamadhammoud merged commit 119b73f into develop Apr 8, 2026
2 checks passed
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.

3 participants