Skip to content

243 aggregator signature aggregator mixes batches making the agg response fail#245

Merged
MauroToscano merged 10 commits intomainfrom
243-aggregator-signature-aggregator-mixes-batches-making-the-agg-response-fail
May 30, 2024
Merged

243 aggregator signature aggregator mixes batches making the agg response fail#245
MauroToscano merged 10 commits intomainfrom
243-aggregator-signature-aggregator-mixes-batches-making-the-agg-response-fail

Conversation

@MauroToscano
Copy link
Copy Markdown
Contributor

@MauroToscano MauroToscano commented May 29, 2024

Fixed the bug by having maps with Roots and Indexes, and not assuming the Signature is always working over the last element.

Additionally I removed some data that wasn't being used or signed from the SignedResponse of the operator, changed the contract pragma so it can compile, and renamed a variable from task to batch for consistency

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 29, 2024

Changes to gas cost

Generated at commit: 76618b24b9fd930756b3d5b97df911a2813c0e1d, compared to commit: 65db9761d7836b083efcbe0aa86b4ebe43da9d6f

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
TransparentUpgradeableProxy blsApkRegistry
stakeRegistry
+21 ❌
+21 ❌
+1.87%
+0.28%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
TransparentUpgradeableProxy 592,648 (+19,654) blsApkRegistry
delegation
initialize
setBLSPublicKey
stakeRegistry
1,143 (+21)
7,643 (+21)
76,951 (+21)
119,146 (+14)
7,623 (+21)
+1.87%
+0.28%
+0.03%
+0.01%
+0.28%
1,143 (+21)
7,643 (+21)
76,951 (+21)
119,146 (+14)
7,623 (+21)
+1.87%
+0.28%
+0.03%
+0.01%
+0.28%
1,143 (+21)
7,643 (+21)
76,951 (+21)
119,146 (+14)
7,623 (+21)
+1.87%
+0.28%
+0.03%
+0.01%
+0.28%
1,143 (+21)
7,643 (+21)
76,951 (+21)
119,146 (+14)
7,623 (+21)
+1.87%
+0.28%
+0.03%
+0.01%
+0.28%
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
RegistryCoordinatorHarness 6,099,207 (+6,161) initialize 54,751,399 (+33,604) +0.06% 54,751,399 (+33,604) +0.06% 54,751,399 (+33,604) +0.06% 54,751,399 (+33,604) +0.06% 1 (0)
ProxyAdmin 443,175 (+16) upgrade
upgradeAndCall
38,830 (+21)
55,317,142 (+33,712)
+0.05%
+0.06%
38,839 (+21)
55,317,142 (+33,712)
+0.05%
+0.06%
38,842 (+21)
55,317,142 (+33,712)
+0.05%
+0.06%
38,842 (+21)
55,317,142 (+33,712)
+0.05%
+0.06%
4 (0)
1 (0)
StakeRegistryHarness 3,225,706 (+37,779) initializeQuorum 143,154 (+53) +0.04% 162,950 (+53) +0.03% 163,054 (+53) +0.03% 163,054 (+53) +0.03% 192 (0)
BLSApkRegistryHarness 1,867,561 (+29,948) setBLSPublicKey 89,365 (-7) -0.01% 89,365 (-7) -0.01% 89,365 (-7) -0.01% 89,365 (-7) -0.01% 1 (0)
AlignedLayerServiceManager 3,622,999 (+25,055) createNewTask 48,329 (+2) +0.00% 48,528 (+2) +0.00% 48,437 (+2) +0.00% 49,067 (+2) +0.00% 256 (0)
AVSDirectory 1,740,390 (+1,343)
Slasher 852,269 (+3,071)
StrategyManagerMock 1,405,102 (-4,538)
IndexRegistry 1,103,762 (+16,825)
ServiceManagerMock 1,225,766 (+15,617)

@MauroToscano MauroToscano requested a review from entropidelic May 29, 2024 22:14
@MauroToscano MauroToscano marked this pull request as ready for review May 29, 2024 22:14
Comment thread aggregator/internal/pkg/aggregator.go Outdated
Comment thread contracts/src/core/ERC20Mock.sol
Copy link
Copy Markdown
Contributor

@entropidelic entropidelic left a comment

Choose a reason for hiding this comment

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

lgtm, just left some small nits

Copy link
Copy Markdown
Contributor

@NicolasRampoldi NicolasRampoldi left a comment

Choose a reason for hiding this comment

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

lgtm, tested it and it works as expected

@MauroToscano MauroToscano merged commit fc8b45c into main May 30, 2024
@MauroToscano MauroToscano deleted the 243-aggregator-signature-aggregator-mixes-batches-making-the-agg-response-fail branch May 30, 2024 17:43
MauroToscano added a commit that referenced this pull request May 30, 2024
…onse fail (#245)

Co-authored-by: Nicolas Rampoldi <58613770+NicolasRampoldi@users.noreply.github.com>
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.

( Aggregator ): Signature aggregator mixes batches, making the agg response fail

3 participants