Skip to content

feat: make signing eip 712 compliant - #916

Merged
MauroToscano merged 29 commits into
stagingfrom
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged

feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom
feat-make-signing-eip-712-compliant

Conversation

@NicolasRampoldi

@NicolasRampoldiNicolasRampoldi commented Sep 3, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

Important

Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.

Important

The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.

Note

The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.

Note

As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.

If you want to read more about the EIP-712.

Description

  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:

    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying

Devnet:

make anvil_add_type_hash_to_batcher_payment_service

Testnet:

  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file

make upgrade_add_type_hash

Working with Metamask

Screen.Recording.2024-09-09.at.15.22.42.mov

To Test

  • Run make deps.
  • Run everything as usual and make sure it is working properly.

@NicolasRampoldiNicolasRampoldi linked an issue Sep 3, 2024 that may be closed by this pull request
Make signing EIP-712 compliant #877
Closed
@NicolasRampoldiNicolasRampoldi self-assigned this Sep 3, 2024
@github-actions

github-actionsBot commented Sep 3, 2024•
edited
Loading

Copy link
Copy Markdown

Changes to gas cost

Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d

🧾 Summary (10% most significant diffs)

ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)

@NicolasRampoldi
NicolasRampoldi marked this pull request as ready for review September 3, 2024 19:44
taturosati
taturosati reviewed Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs Outdated
Comment threadbatcher/aligned-sdk/src/core/types.rs Outdated
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati suggested changes Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs Outdated
entropidelic
entropidelic reviewed Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs Outdated
uri-99
uri-99 suggested changes Sep 6, 2024
View reviewed changes

@uri-99uri-99 left a comment

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.

Didn't finish the review but I leave you this request changes on the mean time

Comment threadcontracts/src/core/BatcherPaymentService.sol Outdated
@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic previously approved these changes Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99 reviewed Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano requested changes Sep 9, 2024
View reviewed changes

@MauroToscanoMauroToscano left a comment

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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code

@MauroToscano
MauroToscano merged commit dd19e5f into stagingSep 10, 2024
@MauroToscano
MauroToscano deleted the feat-make-signing-eip-712-compliant branch September 10, 2024 19:21
@MauroToscanoMauroToscano mentioned this pull request Sep 11, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati
+2 more reviewers
@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant

5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat: make signing eip 712 compliant by NicolasRampoldi · Pull Request #916 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: make signing eip 712 compliant - #916
Merged
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged
feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Conversation
@NicolasRampoldi
@NicolasRampoldiNicolasRampoldi
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
Important
Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.
Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:
    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
  • Run make deps.
  • Run everything as usual and make sure it is working properly.
@NicolasRampoldiNicolasRampoldi
linked an issue
Sep 3, 2024
that may be
closed
by this pull request


Make signing EIP-712 compliant
#877

Closed

@NicolasRampoldiNicolasRampoldi
self-assigned this
Sep 3, 2024
@github-actions
github-actionsBot
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)
@NicolasRampoldi
NicolasRampoldi
marked this pull request as ready for review
September 3, 2024 19:44
taturosati
taturosati
reviewed
Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati
suggested changes
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

entropidelic
entropidelic
reviewed
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

uri-99
uri-99
suggested changes
Sep 6, 2024
View reviewed changes
@uri-99uri-99
left a comment


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.

Didn't finish the review but I leave you this request changes on the mean time
Comment threadcontracts/src/core/BatcherPaymentService.sol
Outdated

@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic
previously approved these changes
Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99
reviewed
Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano
requested changes
Sep 9, 2024
View reviewed changes
@MauroToscanoMauroToscano
left a comment


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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
@MauroToscano
MauroToscano
merged commit dd19e5f
into
stagingSep 10, 2024
@MauroToscano
MauroToscano
deleted the

feat-make-signing-eip-712-compliant

branch
September 10, 2024 19:21
@MauroToscanoMauroToscano
mentioned this pull request
Sep 11, 2024

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati

+2 more reviewers

@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant


5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat: make signing eip 712 compliant by NicolasRampoldi · Pull Request #916 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: make signing eip 712 compliant - #916
Merged
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged
feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Conversation
@NicolasRampoldi
@NicolasRampoldiNicolasRampoldi
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
Important
Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.
Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:
    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
  • Run make deps.
  • Run everything as usual and make sure it is working properly.
@NicolasRampoldiNicolasRampoldi
linked an issue
Sep 3, 2024
that may be
closed
by this pull request


Make signing EIP-712 compliant
#877

Closed

@NicolasRampoldiNicolasRampoldi
self-assigned this
Sep 3, 2024
@github-actions
github-actionsBot
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)
@NicolasRampoldi
NicolasRampoldi
marked this pull request as ready for review
September 3, 2024 19:44
taturosati
taturosati
reviewed
Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati
suggested changes
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

entropidelic
entropidelic
reviewed
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

uri-99
uri-99
suggested changes
Sep 6, 2024
View reviewed changes
@uri-99uri-99
left a comment


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.

Didn't finish the review but I leave you this request changes on the mean time
Comment threadcontracts/src/core/BatcherPaymentService.sol
Outdated

@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic
previously approved these changes
Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99
reviewed
Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano
requested changes
Sep 9, 2024
View reviewed changes
@MauroToscanoMauroToscano
left a comment


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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
@MauroToscano
MauroToscano
merged commit dd19e5f
into
stagingSep 10, 2024
@MauroToscano
MauroToscano
deleted the

feat-make-signing-eip-712-compliant

branch
September 10, 2024 19:21
@MauroToscanoMauroToscano
mentioned this pull request
Sep 11, 2024

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati

+2 more reviewers

@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant


5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat: make signing eip 712 compliant by NicolasRampoldi · Pull Request #916 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: make signing eip 712 compliant - #916
Merged
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged
feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Conversation
@NicolasRampoldi
@NicolasRampoldiNicolasRampoldi
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
Important
Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.
Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:
    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
  • Run make deps.
  • Run everything as usual and make sure it is working properly.
@NicolasRampoldiNicolasRampoldi
linked an issue
Sep 3, 2024
that may be
closed
by this pull request


Make signing EIP-712 compliant
#877

Closed

@NicolasRampoldiNicolasRampoldi
self-assigned this
Sep 3, 2024
@github-actions
github-actionsBot
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)
@NicolasRampoldi
NicolasRampoldi
marked this pull request as ready for review
September 3, 2024 19:44
taturosati
taturosati
reviewed
Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati
suggested changes
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

entropidelic
entropidelic
reviewed
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

uri-99
uri-99
suggested changes
Sep 6, 2024
View reviewed changes
@uri-99uri-99
left a comment


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.

Didn't finish the review but I leave you this request changes on the mean time
Comment threadcontracts/src/core/BatcherPaymentService.sol
Outdated

@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic
previously approved these changes
Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99
reviewed
Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano
requested changes
Sep 9, 2024
View reviewed changes
@MauroToscanoMauroToscano
left a comment


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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
@MauroToscano
MauroToscano
merged commit dd19e5f
into
stagingSep 10, 2024
@MauroToscano
MauroToscano
deleted the

feat-make-signing-eip-712-compliant

branch
September 10, 2024 19:21
@MauroToscanoMauroToscano
mentioned this pull request
Sep 11, 2024

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati

+2 more reviewers

@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant


5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
feat: make signing eip 712 compliant by NicolasRampoldi · Pull Request #916 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: make signing eip 712 compliant - #916
Merged
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged
feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Conversation
@NicolasRampoldi
@NicolasRampoldiNicolasRampoldi
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
Important
Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.
Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:
    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
  • Run make deps.
  • Run everything as usual and make sure it is working properly.
@NicolasRampoldiNicolasRampoldi
linked an issue
Sep 3, 2024
that may be
closed
by this pull request


Make signing EIP-712 compliant
#877

Closed

@NicolasRampoldiNicolasRampoldi
self-assigned this
Sep 3, 2024
@github-actions
github-actionsBot
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)
@NicolasRampoldi
NicolasRampoldi
marked this pull request as ready for review
September 3, 2024 19:44
taturosati
taturosati
reviewed
Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati
suggested changes
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

entropidelic
entropidelic
reviewed
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

uri-99
uri-99
suggested changes
Sep 6, 2024
View reviewed changes
@uri-99uri-99
left a comment


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.

Didn't finish the review but I leave you this request changes on the mean time
Comment threadcontracts/src/core/BatcherPaymentService.sol
Outdated

@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic
previously approved these changes
Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99
reviewed
Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano
requested changes
Sep 9, 2024
View reviewed changes
@MauroToscanoMauroToscano
left a comment


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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
@MauroToscano
MauroToscano
merged commit dd19e5f
into
stagingSep 10, 2024
@MauroToscano
MauroToscano
deleted the

feat-make-signing-eip-712-compliant

branch
September 10, 2024 19:21
@MauroToscanoMauroToscano
mentioned this pull request
Sep 11, 2024

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati

+2 more reviewers

@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant


5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat: make signing eip 712 compliant by NicolasRampoldi · Pull Request #916 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: make signing eip 712 compliant - #916
Merged
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged
feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Conversation
@NicolasRampoldi
@NicolasRampoldiNicolasRampoldi
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
Important
Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.
Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:
    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
  • Run make deps.
  • Run everything as usual and make sure it is working properly.
@NicolasRampoldiNicolasRampoldi
linked an issue
Sep 3, 2024
that may be
closed
by this pull request


Make signing EIP-712 compliant
#877

Closed

@NicolasRampoldiNicolasRampoldi
self-assigned this
Sep 3, 2024
@github-actions
github-actionsBot
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)
@NicolasRampoldi
NicolasRampoldi
marked this pull request as ready for review
September 3, 2024 19:44
taturosati
taturosati
reviewed
Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati
suggested changes
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

entropidelic
entropidelic
reviewed
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

uri-99
uri-99
suggested changes
Sep 6, 2024
View reviewed changes
@uri-99uri-99
left a comment


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.

Didn't finish the review but I leave you this request changes on the mean time
Comment threadcontracts/src/core/BatcherPaymentService.sol
Outdated

@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic
previously approved these changes
Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99
reviewed
Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano
requested changes
Sep 9, 2024
View reviewed changes
@MauroToscanoMauroToscano
left a comment


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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
@MauroToscano
MauroToscano
merged commit dd19e5f
into
stagingSep 10, 2024
@MauroToscano
MauroToscano
deleted the

feat-make-signing-eip-712-compliant

branch
September 10, 2024 19:21
@MauroToscanoMauroToscano
mentioned this pull request
Sep 11, 2024

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati

+2 more reviewers

@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant


5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat: make signing eip 712 compliant by NicolasRampoldi · Pull Request #916 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: make signing eip 712 compliant - #916
Merged
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged
feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Conversation
@NicolasRampoldi
@NicolasRampoldiNicolasRampoldi
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
Important
Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.
Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:
    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
  • Run make deps.
  • Run everything as usual and make sure it is working properly.
@NicolasRampoldiNicolasRampoldi
linked an issue
Sep 3, 2024
that may be
closed
by this pull request


Make signing EIP-712 compliant
#877

Closed

@NicolasRampoldiNicolasRampoldi
self-assigned this
Sep 3, 2024
@github-actions
github-actionsBot
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)
@NicolasRampoldi
NicolasRampoldi
marked this pull request as ready for review
September 3, 2024 19:44
taturosati
taturosati
reviewed
Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati
suggested changes
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

entropidelic
entropidelic
reviewed
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

uri-99
uri-99
suggested changes
Sep 6, 2024
View reviewed changes
@uri-99uri-99
left a comment


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.

Didn't finish the review but I leave you this request changes on the mean time
Comment threadcontracts/src/core/BatcherPaymentService.sol
Outdated

@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic
previously approved these changes
Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99
reviewed
Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano
requested changes
Sep 9, 2024
View reviewed changes
@MauroToscanoMauroToscano
left a comment


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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
@MauroToscano
MauroToscano
merged commit dd19e5f
into
stagingSep 10, 2024
@MauroToscano
MauroToscano
deleted the

feat-make-signing-eip-712-compliant

branch
September 10, 2024 19:21
@MauroToscanoMauroToscano
mentioned this pull request
Sep 11, 2024

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati

+2 more reviewers

@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant


5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
feat: make signing eip 712 compliant by NicolasRampoldi · Pull Request #916 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: make signing eip 712 compliant - #916
Merged
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Sep 10, 2024
Merged
feat: make signing eip 712 compliant#916
MauroToscano merged 29 commits into
stagingfrom 
feat-make-signing-eip-712-compliant
Conversation
@NicolasRampoldi
@NicolasRampoldiNicolasRampoldi
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
Important
Added chain param to submit and submit_multiple in the SDK. This is needed to get the BatcherPaymentService contract address.
Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
  • This PR makes the NoncedVerificationData signing EIP712 compliant. The EIP712Domain has:
    name: Aligned
version: 1
chain_id: <current_chain_id>
verifying_contract: <current_payment_service contract_addr>

Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
  • Stage or prod variables should be set accordingly on the contracts/scripts/.env file
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
  • Run make deps.
  • Run everything as usual and make sure it is working properly.
@NicolasRampoldiNicolasRampoldi
linked an issue
Sep 3, 2024
that may be
closed
by this pull request


Make signing EIP-712 compliant
#877

Closed

@NicolasRampoldiNicolasRampoldi
self-assigned this
Sep 3, 2024
@github-actions
github-actionsBot
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: af42f188dc0ab725b46a4ee2bf8890f89e239981, compared to commit: 42cc5f549f209d15db31771119b017b39ba5d05d
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
TransparentUpgradeableProxyblsApkRegistry
delegation
stakeRegistry		-21 ✅
-21 ✅
-21 ✅		-1.91%
-0.27%
-0.27%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
TransparentUpgradeableProxy573,006 (-19,642)blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1,080 (-21)
7,622 (-21)
101,061 (-21)
119,132 (-14)
7,646 (-21)		-1.91%
-0.27%
-0.02%
-0.01%
-0.27%		1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
AlignedLayerServiceManager4,648,017 (-19,192)createNewTask
receive		56,967 (+12)
21,169 (+6)		+0.02%
+0.03%		76,963 (+57)
44,783 (+10)		+0.07%
+0.02%		77,035 (0)
45,064 (+11)		0.00%
+0.02%		78,140 (-72)
45,064 (+11)		-0.09%
+0.02%		256 (0)
256 (0)
RegistryCoordinatorHarness5,828,753 (-1,796)initialize54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%54,717,816 (-33,604)-0.06%1 (0)
ProxyAdmin443,159 (-16)upgrade
upgradeAndCall		38,809 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,818 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		38,821 (-21)
55,283,451 (-33,712)		-0.05%
-0.06%		4 (0)
1 (0)
StakeRegistryHarness3,187,927 (-37,767)initializeQuorum143,101 (-53)-0.04%162,897 (-53)-0.03%163,001 (-53)-0.03%163,001 (-53)-0.03%192 (0)
BLSApkRegistryHarness1,837,601 (-29,960)setBLSPublicKey89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%89,372 (+7)+0.01%1 (0)
AVSDirectory1,739,047 (-1,343)
Slasher849,198 (-3,071)
StrategyManagerMock1,270,846 (+4,755)
IndexRegistry1,086,937 (-16,825)
ServiceManagerMock1,611,464 (-16,658)
@NicolasRampoldi
NicolasRampoldi
marked this pull request as ready for review
September 3, 2024 19:44
taturosati
taturosati
reviewed
Sep 3, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
801f690
# Conflicts:
#	batcher/aligned-sdk/src/core/types.rs
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge
b82987e
@NicolasRampoldi
chore: bump up protocol version
83d8d77
taturosati
taturosati
suggested changes
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

entropidelic
entropidelic
reviewed
Sep 5, 2024
View reviewed changes
Comment threadbatcher/aligned-sdk/src/core/types.rs
Outdated

uri-99
uri-99
suggested changes
Sep 6, 2024
View reviewed changes
@uri-99uri-99
left a comment


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.

Didn't finish the review but I leave you this request changes on the mean time
Comment threadcontracts/src/core/BatcherPaymentService.sol
Outdated

@NicolasRampoldi
forge install: openzeppelin-contracts-upgradeable
76ed477
v5.0.2
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
7772e3b
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
entropidelic
entropidelic
previously approved these changes
Sep 6, 2024
View reviewed changes
@NicolasRampoldi
chore: anvil state
2d9e71f
@NicolasRampoldi
feat: add setHashType in contract
4ffd79c
@NicolasRampoldi
Merge branch 'main' into feat-make-signing-eip-712-compliant
ab2b391
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	docs/3_guides/4_generating_proofs.md
@NicolasRampoldi
fix: deployment script
6610aca
uri-99
uri-99
reviewed
Sep 6, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol
MauroToscano
MauroToscano
requested changes
Sep 9, 2024
View reviewed changes
@MauroToscanoMauroToscano
left a comment


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.

Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
@MauroToscano
MauroToscano
merged commit dd19e5f
into
stagingSep 10, 2024
@MauroToscano
MauroToscano
deleted the

feat-make-signing-eip-712-compliant

branch
September 10, 2024 19:21
@MauroToscanoMauroToscano
mentioned this pull request
Sep 11, 2024

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MauroToscanoMauroToscanoMauroToscano approved these changes
@taturosatitaturosatiAwaiting requested review from taturosati

+2 more reviewers

@uri-99uri-99uri-99 approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligence

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Make signing EIP-712 compliant


5 participants

@NicolasRampoldi@MauroToscano@uri-99@taturosati@entropidelic