Skip to content

feat: aggregator should check if gas is enough before respondToTask - #918

Merged
entropidelic merged 131 commits into
stagingfrom
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Sep 16, 2024
Merged

feat: aggregator should check if gas is enough before respondToTask#918
entropidelic merged 131 commits into
stagingfrom
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask

Conversation

@uri-99

@uri-99uri-99 commented Sep 3, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR

  • Aggregator runs a series of new checks before submitting RespondToTask tx:
    • runs a simulation of RespondToTask and checks RespondToTaskFeeLimit is enough
    • checks Batcher has enough funds to pay for RespondToTaskFeeLimit
    • checks Agg has enough funds to pay for RespondToTaskFeeLimit

In this PR, the variable batchersBalances was changed from internal to public; because the aggregator must check how much balance does the Batcher has, to compare it with RespondToTaskFeeLimit.

To Deploy

Aggregator code has changed (adding new checks) , so it must be redeployed.
AlignedLayerServiceManager variable was changed from internal to public, so it must be redeployed.

To Test

Happy Path

make anvil_start_with_block_time

make aggregator_start

make operator_register_and_start

make batcher_start_local

make run_explorer

And when they finish starting up, you can send proofs:

make batcher_send_risc0_burst

Unhappy paths

  • Empty Aggregator balance:
cast send 0x0000000000000000000000000000000000000000 --value 9999998499999944500000 --private-key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a --rpc-url "http://localhost:8545"

Note cast may not show the tx as completed, but you can verify batchers balance with:

cast balance 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 --rpc-url localhost:8545

Then send proofs, Aggregator should not try to submit the tx

make batcher_send_risc0_burst
  • change the DEFAULT_AGGREGATOR_FEE_MULTIPLIER and RESPOND_TO_TASK_FEE_LIMIT_MULTIPLIER to different values, and check it behaves accordingly
  • hardcode respondToTaskFeeLimit values on the BatcherPaymentService or AlignedServiceManagerand, and check it behaves accordingly
  • (These 2 cases are pretty similar but one may be favourable over the other depending which one you understand better and which value you want to test more in detail)

taturosatiand others added 30 commits August 26, 2024 14:35
@taturosati
feat (batcher-payment-service): allow user to specify max fee
1a02d7d
@taturosati
feat (batcher-sdk): allow uesr to specify max fee
e7882fc
@taturosati
refactor: change & specify default max fee
5f4f248
@taturosati
refactor (batcher): use priority queue for batch queue
9fa7610
@taturosati
feat (batcher): check max fees when building batch
89243e2
@taturosati
feat (batcher): check max fee is less for bigger nonce
32de669
@taturosati
refactor (batcher): use priority queue lib
f309627
@taturosati
feat (batcher): allow replacement higher fee
10cb529
@taturosati
chore: merge
3d01edd
@taturosati
fix: solhint
6fadf42
@taturosati
fix: handle cache reset correctly
08c7c89
@taturosati
fix (batcher): handling of replacement fee
d991b62
@taturosati
refactor: extract gas check multiplier to constant
f62da9f
@taturosati
chore: merge
03bd4e6
@taturosati
fix: dont allow size 1 batch
1983185
@taturosati
fix: update ws connection on replacemnt tx
f20679a
@taturosati
docs: max_fee
a81a870
@taturosati
fix: verify max fee
0620d83
@taturosati
chore: add comment on user_min_fee
8e4303a
taturosati
refactor: check nonce and max fee
82cb11f
taturosati
refactor: is_batch_ready
92090ab
@uri-99
chore: add error signature on new error
d94cd6d
taturosati
chore: remove old comment
cc382cd
taturosati
feat: close old sink
6a9b7ac
taturosati
chore: update function docs
eaa9e78
taturosati
chore: merge
1d83e63
taturosati
fix (aligned-sdk): add batcher address to check is batch verifed
70a1482
taturosati
refactor: rename batcher_addr to payment_service_addr on aligned cli
5cca7ea
taturosati
docs: add batcher address to sdk methods
29cfbd0
taturosati
refactor: apply rename suggestions from code review
fb9934d
Base automatically changed from 881-feat-add-limit-on-aggregator-spending to mainSeptember 6, 2024 18:41
NicolasRampoldi
NicolasRampoldi suggested changes Sep 10, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go Outdated
Comment threadcore/chainio/avs_writer.go Outdated
taturosati
taturosati reviewed Sep 11, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Comment threadcore/chainio/avs_writer.go Outdated
@uri-99uri-99 closed this Sep 16, 2024
@uri-99uri-99 reopened this Sep 16, 2024
@github-actions

github-actionsBot commented Sep 16, 2024•
edited
Loading

Copy link
Copy Markdown

Changes to gas cost

Generated at commit: 2c0465a1da5fb2e15359522e421bbcf8d12b5cce, compared to commit: 4fe73dae65b9341ad33a212586564a2a8da8ea3a

🧾 Summary (10% most significant diffs)

ContractMethodAvg (+/-)%
AlignedLayerServiceManagerbatchesState-22 ✅-3.03%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,660,098 (+12,081)batchesState
createNewTask
receive		703 (-22)
56,921 (+2)
21,169 (0)		-3.03%
+0.00%
0.00%		703 (-22)
76,896 (+76)
44,783 (+93)		-3.03%
+0.10%
+0.21%		703 (-22)
76,977 (-46)
45,064 (0)		-3.03%
-0.06%
0.00%		703 (-22)
78,130 (+17)
45,064 (0)		-3.03%
+0.02%
0.00%		256 (0)
256 (0)
256 (0)

@entropidelic
entropidelic merged commit 047a65b into stagingSep 16, 2024
@entropidelic
entropidelic deleted the 882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask branch September 16, 2024 22:16
@uri-99uri-99 mentioned this pull request Sep 23, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NicolasRampoldiNicolasRampoldiAwaiting requested review from NicolasRampoldi
3 more reviewers
@taturosatitaturosatitaturosati left review comments
@PatStilesPatStilesPatStiles approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@uri-99uri-99

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: Aggregator should check if gas is enough before respondToTask

6 participants

@uri-99@PatStiles@NicolasRampoldi@taturosati@entropidelic@glpecile
, '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: aggregator should check if gas is enough before respondToTask by uri-99 · Pull Request #918 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: aggregator should check if gas is enough before respondToTask - #918
Merged
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Sep 16, 2024
Merged
feat: aggregator should check if gas is enough before respondToTask#918
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Conversation
@uri-99
@uri-99uri-99
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR
  • Aggregator runs a series of new checks before submitting RespondToTask tx:

    • runs a simulation of RespondToTask and checks RespondToTaskFeeLimit is enough
    • checks Batcher has enough funds to pay for RespondToTaskFeeLimit
    • checks Agg has enough funds to pay for RespondToTaskFeeLimit
In this PR, the variable batchersBalances was changed from internal to public; because the aggregator must check how much balance does the Batcher has, to compare it with RespondToTaskFeeLimit.
To Deploy
Aggregator code has changed (adding new checks) , so it must be redeployed.

AlignedLayerServiceManager variable was changed from internal to public, so it must be redeployed.
To Test
Happy Path
make anvil_start_with_block_time

make aggregator_start

make operator_register_and_start

make batcher_start_local

make run_explorer

And when they finish starting up, you can send proofs:
make batcher_send_risc0_burst

Unhappy paths
  • Empty Aggregator balance:
cast send 0x0000000000000000000000000000000000000000 --value 9999998499999944500000 --private-key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a --rpc-url "http://localhost:8545"

Note cast may not show the tx as completed, but you can verify batchers balance with:
cast balance 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 --rpc-url localhost:8545

Then send proofs, Aggregator should not try to submit the tx
make batcher_send_risc0_burst

  • change the DEFAULT_AGGREGATOR_FEE_MULTIPLIER and RESPOND_TO_TASK_FEE_LIMIT_MULTIPLIER to different values, and check it behaves accordingly
  • hardcode respondToTaskFeeLimit values on the BatcherPaymentService or AlignedServiceManagerand, and check it behaves accordingly
  • (These 2 cases are pretty similar but one may be favourable over the other depending which one you understand better and which value you want to test more in detail)
taturosatiand others
added 30 commits
August 26, 2024 14:35
@taturosati
feat (batcher-payment-service): allow user to specify max fee
1a02d7d
@taturosati
feat (batcher-sdk): allow uesr to specify max fee
e7882fc
@taturosati
refactor: change & specify default max fee
5f4f248
@taturosati
refactor (batcher): use priority queue for batch queue
9fa7610
@taturosati
feat (batcher): check max fees when building batch
89243e2
@taturosati
feat (batcher): check max fee is less for bigger nonce
32de669
@taturosati
refactor (batcher): use priority queue lib
f309627
@taturosati
feat (batcher): allow replacement higher fee
10cb529
@taturosati
chore: merge
3d01edd
@taturosati
fix: solhint
6fadf42
@taturosati
fix: handle cache reset correctly
08c7c89
@taturosati
fix (batcher): handling of replacement fee
d991b62
@taturosati
refactor: extract gas check multiplier to constant
f62da9f
@taturosati
chore: merge
03bd4e6
@taturosati
fix: dont allow size 1 batch
1983185
@taturosati
fix: update ws connection on replacemnt tx
f20679a
@taturosati
docs: max_fee
a81a870
@taturosati
fix: verify max fee
0620d83
@taturosati
chore: add comment on user_min_fee
8e4303a
taturosati
refactor: check nonce and max fee
82cb11f
taturosati
refactor: is_batch_ready
92090ab
@uri-99
chore: add error signature on new error
d94cd6d
taturosati
chore: remove old comment
cc382cd
taturosati
feat: close old sink
6a9b7ac
taturosati
chore: update function docs
eaa9e78
taturosati
chore: merge
1d83e63
taturosati
fix (aligned-sdk): add batcher address to check is batch verifed
70a1482
taturosati
refactor: rename batcher_addr to payment_service_addr on aligned cli
5cca7ea
taturosati
docs: add batcher address to sdk methods
29cfbd0
taturosati
refactor: apply rename suggestions from code review
fb9934d

Base automatically changed from
881-feat-add-limit-on-aggregator-spending to
mainSeptember 6, 2024 18:41
NicolasRampoldi
NicolasRampoldi
suggested changes
Sep 10, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Outdated

Comment threadcore/chainio/avs_writer.go
Outdated

taturosati
taturosati
reviewed
Sep 11, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Comment threadcore/chainio/avs_writer.go
Outdated

@uri-99uri-99
closed this
Sep 16, 2024
@uri-99uri-99
reopened this
Sep 16, 2024
@github-actions
github-actionsBot
commented
Sep 16, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: 2c0465a1da5fb2e15359522e421bbcf8d12b5cce, compared to commit: 4fe73dae65b9341ad33a212586564a2a8da8ea3a
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
AlignedLayerServiceManagerbatchesState-22 ✅-3.03%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,660,098 (+12,081)batchesState
createNewTask
receive		703 (-22)
56,921 (+2)
21,169 (0)		-3.03%
+0.00%
0.00%		703 (-22)
76,896 (+76)
44,783 (+93)		-3.03%
+0.10%
+0.21%		703 (-22)
76,977 (-46)
45,064 (0)		-3.03%
-0.06%
0.00%		703 (-22)
78,130 (+17)
45,064 (0)		-3.03%
+0.02%
0.00%		256 (0)
256 (0)
256 (0)
@entropidelic
entropidelic
merged commit 047a65b
into
stagingSep 16, 2024
@entropidelic
entropidelic
deleted the

882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask

branch
September 16, 2024 22:16
@uri-99uri-99
mentioned this pull request
Sep 23, 2024

Closed

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

Reviewers

@NicolasRampoldiNicolasRampoldiAwaiting requested review from NicolasRampoldi

3 more reviewers

@taturosatitaturosatitaturosati left review comments
@PatStilesPatStilesPatStiles approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@uri-99uri-99

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

feat: Aggregator should check if gas is enough before respondToTask


6 participants

@uri-99@PatStiles@NicolasRampoldi@taturosati@entropidelic@glpecile

, '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: aggregator should check if gas is enough before respondToTask by uri-99 · Pull Request #918 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: aggregator should check if gas is enough before respondToTask - #918
Merged
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Sep 16, 2024
Merged
feat: aggregator should check if gas is enough before respondToTask#918
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Conversation
@uri-99
@uri-99uri-99
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR
  • Aggregator runs a series of new checks before submitting RespondToTask tx:

    • runs a simulation of RespondToTask and checks RespondToTaskFeeLimit is enough
    • checks Batcher has enough funds to pay for RespondToTaskFeeLimit
    • checks Agg has enough funds to pay for RespondToTaskFeeLimit
In this PR, the variable batchersBalances was changed from internal to public; because the aggregator must check how much balance does the Batcher has, to compare it with RespondToTaskFeeLimit.
To Deploy
Aggregator code has changed (adding new checks) , so it must be redeployed.

AlignedLayerServiceManager variable was changed from internal to public, so it must be redeployed.
To Test
Happy Path
make anvil_start_with_block_time

make aggregator_start

make operator_register_and_start

make batcher_start_local

make run_explorer

And when they finish starting up, you can send proofs:
make batcher_send_risc0_burst

Unhappy paths
  • Empty Aggregator balance:
cast send 0x0000000000000000000000000000000000000000 --value 9999998499999944500000 --private-key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a --rpc-url "http://localhost:8545"

Note cast may not show the tx as completed, but you can verify batchers balance with:
cast balance 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 --rpc-url localhost:8545

Then send proofs, Aggregator should not try to submit the tx
make batcher_send_risc0_burst

  • change the DEFAULT_AGGREGATOR_FEE_MULTIPLIER and RESPOND_TO_TASK_FEE_LIMIT_MULTIPLIER to different values, and check it behaves accordingly
  • hardcode respondToTaskFeeLimit values on the BatcherPaymentService or AlignedServiceManagerand, and check it behaves accordingly
  • (These 2 cases are pretty similar but one may be favourable over the other depending which one you understand better and which value you want to test more in detail)
taturosatiand others
added 30 commits
August 26, 2024 14:35
@taturosati
feat (batcher-payment-service): allow user to specify max fee
1a02d7d
@taturosati
feat (batcher-sdk): allow uesr to specify max fee
e7882fc
@taturosati
refactor: change & specify default max fee
5f4f248
@taturosati
refactor (batcher): use priority queue for batch queue
9fa7610
@taturosati
feat (batcher): check max fees when building batch
89243e2
@taturosati
feat (batcher): check max fee is less for bigger nonce
32de669
@taturosati
refactor (batcher): use priority queue lib
f309627
@taturosati
feat (batcher): allow replacement higher fee
10cb529
@taturosati
chore: merge
3d01edd
@taturosati
fix: solhint
6fadf42
@taturosati
fix: handle cache reset correctly
08c7c89
@taturosati
fix (batcher): handling of replacement fee
d991b62
@taturosati
refactor: extract gas check multiplier to constant
f62da9f
@taturosati
chore: merge
03bd4e6
@taturosati
fix: dont allow size 1 batch
1983185
@taturosati
fix: update ws connection on replacemnt tx
f20679a
@taturosati
docs: max_fee
a81a870
@taturosati
fix: verify max fee
0620d83
@taturosati
chore: add comment on user_min_fee
8e4303a
taturosati
refactor: check nonce and max fee
82cb11f
taturosati
refactor: is_batch_ready
92090ab
@uri-99
chore: add error signature on new error
d94cd6d
taturosati
chore: remove old comment
cc382cd
taturosati
feat: close old sink
6a9b7ac
taturosati
chore: update function docs
eaa9e78
taturosati
chore: merge
1d83e63
taturosati
fix (aligned-sdk): add batcher address to check is batch verifed
70a1482
taturosati
refactor: rename batcher_addr to payment_service_addr on aligned cli
5cca7ea
taturosati
docs: add batcher address to sdk methods
29cfbd0
taturosati
refactor: apply rename suggestions from code review
fb9934d

Base automatically changed from
881-feat-add-limit-on-aggregator-spending to
mainSeptember 6, 2024 18:41
NicolasRampoldi
NicolasRampoldi
suggested changes
Sep 10, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Outdated

Comment threadcore/chainio/avs_writer.go
Outdated

taturosati
taturosati
reviewed
Sep 11, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Comment threadcore/chainio/avs_writer.go
Outdated

@uri-99uri-99
closed this
Sep 16, 2024
@uri-99uri-99
reopened this
Sep 16, 2024
@github-actions
github-actionsBot
commented
Sep 16, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: 2c0465a1da5fb2e15359522e421bbcf8d12b5cce, compared to commit: 4fe73dae65b9341ad33a212586564a2a8da8ea3a
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
AlignedLayerServiceManagerbatchesState-22 ✅-3.03%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,660,098 (+12,081)batchesState
createNewTask
receive		703 (-22)
56,921 (+2)
21,169 (0)		-3.03%
+0.00%
0.00%		703 (-22)
76,896 (+76)
44,783 (+93)		-3.03%
+0.10%
+0.21%		703 (-22)
76,977 (-46)
45,064 (0)		-3.03%
-0.06%
0.00%		703 (-22)
78,130 (+17)
45,064 (0)		-3.03%
+0.02%
0.00%		256 (0)
256 (0)
256 (0)
@entropidelic
entropidelic
merged commit 047a65b
into
stagingSep 16, 2024
@entropidelic
entropidelic
deleted the

882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask

branch
September 16, 2024 22:16
@uri-99uri-99
mentioned this pull request
Sep 23, 2024

Closed

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

Reviewers

@NicolasRampoldiNicolasRampoldiAwaiting requested review from NicolasRampoldi

3 more reviewers

@taturosatitaturosatitaturosati left review comments
@PatStilesPatStilesPatStiles approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@uri-99uri-99

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

feat: Aggregator should check if gas is enough before respondToTask


6 participants

@uri-99@PatStiles@NicolasRampoldi@taturosati@entropidelic@glpecile

, '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: aggregator should check if gas is enough before respondToTask by uri-99 · Pull Request #918 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: aggregator should check if gas is enough before respondToTask - #918
Merged
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Sep 16, 2024
Merged
feat: aggregator should check if gas is enough before respondToTask#918
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Conversation
@uri-99
@uri-99uri-99
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR
  • Aggregator runs a series of new checks before submitting RespondToTask tx:

    • runs a simulation of RespondToTask and checks RespondToTaskFeeLimit is enough
    • checks Batcher has enough funds to pay for RespondToTaskFeeLimit
    • checks Agg has enough funds to pay for RespondToTaskFeeLimit
In this PR, the variable batchersBalances was changed from internal to public; because the aggregator must check how much balance does the Batcher has, to compare it with RespondToTaskFeeLimit.
To Deploy
Aggregator code has changed (adding new checks) , so it must be redeployed.

AlignedLayerServiceManager variable was changed from internal to public, so it must be redeployed.
To Test
Happy Path
make anvil_start_with_block_time

make aggregator_start

make operator_register_and_start

make batcher_start_local

make run_explorer

And when they finish starting up, you can send proofs:
make batcher_send_risc0_burst

Unhappy paths
  • Empty Aggregator balance:
cast send 0x0000000000000000000000000000000000000000 --value 9999998499999944500000 --private-key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a --rpc-url "http://localhost:8545"

Note cast may not show the tx as completed, but you can verify batchers balance with:
cast balance 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 --rpc-url localhost:8545

Then send proofs, Aggregator should not try to submit the tx
make batcher_send_risc0_burst

  • change the DEFAULT_AGGREGATOR_FEE_MULTIPLIER and RESPOND_TO_TASK_FEE_LIMIT_MULTIPLIER to different values, and check it behaves accordingly
  • hardcode respondToTaskFeeLimit values on the BatcherPaymentService or AlignedServiceManagerand, and check it behaves accordingly
  • (These 2 cases are pretty similar but one may be favourable over the other depending which one you understand better and which value you want to test more in detail)
taturosatiand others
added 30 commits
August 26, 2024 14:35
@taturosati
feat (batcher-payment-service): allow user to specify max fee
1a02d7d
@taturosati
feat (batcher-sdk): allow uesr to specify max fee
e7882fc
@taturosati
refactor: change & specify default max fee
5f4f248
@taturosati
refactor (batcher): use priority queue for batch queue
9fa7610
@taturosati
feat (batcher): check max fees when building batch
89243e2
@taturosati
feat (batcher): check max fee is less for bigger nonce
32de669
@taturosati
refactor (batcher): use priority queue lib
f309627
@taturosati
feat (batcher): allow replacement higher fee
10cb529
@taturosati
chore: merge
3d01edd
@taturosati
fix: solhint
6fadf42
@taturosati
fix: handle cache reset correctly
08c7c89
@taturosati
fix (batcher): handling of replacement fee
d991b62
@taturosati
refactor: extract gas check multiplier to constant
f62da9f
@taturosati
chore: merge
03bd4e6
@taturosati
fix: dont allow size 1 batch
1983185
@taturosati
fix: update ws connection on replacemnt tx
f20679a
@taturosati
docs: max_fee
a81a870
@taturosati
fix: verify max fee
0620d83
@taturosati
chore: add comment on user_min_fee
8e4303a
taturosati
refactor: check nonce and max fee
82cb11f
taturosati
refactor: is_batch_ready
92090ab
@uri-99
chore: add error signature on new error
d94cd6d
taturosati
chore: remove old comment
cc382cd
taturosati
feat: close old sink
6a9b7ac
taturosati
chore: update function docs
eaa9e78
taturosati
chore: merge
1d83e63
taturosati
fix (aligned-sdk): add batcher address to check is batch verifed
70a1482
taturosati
refactor: rename batcher_addr to payment_service_addr on aligned cli
5cca7ea
taturosati
docs: add batcher address to sdk methods
29cfbd0
taturosati
refactor: apply rename suggestions from code review
fb9934d

Base automatically changed from
881-feat-add-limit-on-aggregator-spending to
mainSeptember 6, 2024 18:41
NicolasRampoldi
NicolasRampoldi
suggested changes
Sep 10, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Outdated

Comment threadcore/chainio/avs_writer.go
Outdated

taturosati
taturosati
reviewed
Sep 11, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Comment threadcore/chainio/avs_writer.go
Outdated

@uri-99uri-99
closed this
Sep 16, 2024
@uri-99uri-99
reopened this
Sep 16, 2024
@github-actions
github-actionsBot
commented
Sep 16, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: 2c0465a1da5fb2e15359522e421bbcf8d12b5cce, compared to commit: 4fe73dae65b9341ad33a212586564a2a8da8ea3a
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
AlignedLayerServiceManagerbatchesState-22 ✅-3.03%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,660,098 (+12,081)batchesState
createNewTask
receive		703 (-22)
56,921 (+2)
21,169 (0)		-3.03%
+0.00%
0.00%		703 (-22)
76,896 (+76)
44,783 (+93)		-3.03%
+0.10%
+0.21%		703 (-22)
76,977 (-46)
45,064 (0)		-3.03%
-0.06%
0.00%		703 (-22)
78,130 (+17)
45,064 (0)		-3.03%
+0.02%
0.00%		256 (0)
256 (0)
256 (0)
@entropidelic
entropidelic
merged commit 047a65b
into
stagingSep 16, 2024
@entropidelic
entropidelic
deleted the

882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask

branch
September 16, 2024 22:16
@uri-99uri-99
mentioned this pull request
Sep 23, 2024

Closed

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

Reviewers

@NicolasRampoldiNicolasRampoldiAwaiting requested review from NicolasRampoldi

3 more reviewers

@taturosatitaturosatitaturosati left review comments
@PatStilesPatStilesPatStiles approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@uri-99uri-99

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

feat: Aggregator should check if gas is enough before respondToTask


6 participants

@uri-99@PatStiles@NicolasRampoldi@taturosati@entropidelic@glpecile

, '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: aggregator should check if gas is enough before respondToTask by uri-99 · Pull Request #918 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: aggregator should check if gas is enough before respondToTask - #918
Merged
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Sep 16, 2024
Merged
feat: aggregator should check if gas is enough before respondToTask#918
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Conversation
@uri-99
@uri-99uri-99
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR
  • Aggregator runs a series of new checks before submitting RespondToTask tx:

    • runs a simulation of RespondToTask and checks RespondToTaskFeeLimit is enough
    • checks Batcher has enough funds to pay for RespondToTaskFeeLimit
    • checks Agg has enough funds to pay for RespondToTaskFeeLimit
In this PR, the variable batchersBalances was changed from internal to public; because the aggregator must check how much balance does the Batcher has, to compare it with RespondToTaskFeeLimit.
To Deploy
Aggregator code has changed (adding new checks) , so it must be redeployed.

AlignedLayerServiceManager variable was changed from internal to public, so it must be redeployed.
To Test
Happy Path
make anvil_start_with_block_time

make aggregator_start

make operator_register_and_start

make batcher_start_local

make run_explorer

And when they finish starting up, you can send proofs:
make batcher_send_risc0_burst

Unhappy paths
  • Empty Aggregator balance:
cast send 0x0000000000000000000000000000000000000000 --value 9999998499999944500000 --private-key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a --rpc-url "http://localhost:8545"

Note cast may not show the tx as completed, but you can verify batchers balance with:
cast balance 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 --rpc-url localhost:8545

Then send proofs, Aggregator should not try to submit the tx
make batcher_send_risc0_burst

  • change the DEFAULT_AGGREGATOR_FEE_MULTIPLIER and RESPOND_TO_TASK_FEE_LIMIT_MULTIPLIER to different values, and check it behaves accordingly
  • hardcode respondToTaskFeeLimit values on the BatcherPaymentService or AlignedServiceManagerand, and check it behaves accordingly
  • (These 2 cases are pretty similar but one may be favourable over the other depending which one you understand better and which value you want to test more in detail)
taturosatiand others
added 30 commits
August 26, 2024 14:35
@taturosati
feat (batcher-payment-service): allow user to specify max fee
1a02d7d
@taturosati
feat (batcher-sdk): allow uesr to specify max fee
e7882fc
@taturosati
refactor: change & specify default max fee
5f4f248
@taturosati
refactor (batcher): use priority queue for batch queue
9fa7610
@taturosati
feat (batcher): check max fees when building batch
89243e2
@taturosati
feat (batcher): check max fee is less for bigger nonce
32de669
@taturosati
refactor (batcher): use priority queue lib
f309627
@taturosati
feat (batcher): allow replacement higher fee
10cb529
@taturosati
chore: merge
3d01edd
@taturosati
fix: solhint
6fadf42
@taturosati
fix: handle cache reset correctly
08c7c89
@taturosati
fix (batcher): handling of replacement fee
d991b62
@taturosati
refactor: extract gas check multiplier to constant
f62da9f
@taturosati
chore: merge
03bd4e6
@taturosati
fix: dont allow size 1 batch
1983185
@taturosati
fix: update ws connection on replacemnt tx
f20679a
@taturosati
docs: max_fee
a81a870
@taturosati
fix: verify max fee
0620d83
@taturosati
chore: add comment on user_min_fee
8e4303a
taturosati
refactor: check nonce and max fee
82cb11f
taturosati
refactor: is_batch_ready
92090ab
@uri-99
chore: add error signature on new error
d94cd6d
taturosati
chore: remove old comment
cc382cd
taturosati
feat: close old sink
6a9b7ac
taturosati
chore: update function docs
eaa9e78
taturosati
chore: merge
1d83e63
taturosati
fix (aligned-sdk): add batcher address to check is batch verifed
70a1482
taturosati
refactor: rename batcher_addr to payment_service_addr on aligned cli
5cca7ea
taturosati
docs: add batcher address to sdk methods
29cfbd0
taturosati
refactor: apply rename suggestions from code review
fb9934d

Base automatically changed from
881-feat-add-limit-on-aggregator-spending to
mainSeptember 6, 2024 18:41
NicolasRampoldi
NicolasRampoldi
suggested changes
Sep 10, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Outdated

Comment threadcore/chainio/avs_writer.go
Outdated

taturosati
taturosati
reviewed
Sep 11, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Comment threadcore/chainio/avs_writer.go
Outdated

@uri-99uri-99
closed this
Sep 16, 2024
@uri-99uri-99
reopened this
Sep 16, 2024
@github-actions
github-actionsBot
commented
Sep 16, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: 2c0465a1da5fb2e15359522e421bbcf8d12b5cce, compared to commit: 4fe73dae65b9341ad33a212586564a2a8da8ea3a
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
AlignedLayerServiceManagerbatchesState-22 ✅-3.03%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,660,098 (+12,081)batchesState
createNewTask
receive		703 (-22)
56,921 (+2)
21,169 (0)		-3.03%
+0.00%
0.00%		703 (-22)
76,896 (+76)
44,783 (+93)		-3.03%
+0.10%
+0.21%		703 (-22)
76,977 (-46)
45,064 (0)		-3.03%
-0.06%
0.00%		703 (-22)
78,130 (+17)
45,064 (0)		-3.03%
+0.02%
0.00%		256 (0)
256 (0)
256 (0)
@entropidelic
entropidelic
merged commit 047a65b
into
stagingSep 16, 2024
@entropidelic
entropidelic
deleted the

882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask

branch
September 16, 2024 22:16
@uri-99uri-99
mentioned this pull request
Sep 23, 2024

Closed

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

Reviewers

@NicolasRampoldiNicolasRampoldiAwaiting requested review from NicolasRampoldi

3 more reviewers

@taturosatitaturosatitaturosati left review comments
@PatStilesPatStilesPatStiles approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@uri-99uri-99

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

feat: Aggregator should check if gas is enough before respondToTask


6 participants

@uri-99@PatStiles@NicolasRampoldi@taturosati@entropidelic@glpecile

, '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: aggregator should check if gas is enough before respondToTask by uri-99 · Pull Request #918 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: aggregator should check if gas is enough before respondToTask - #918
Merged
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Sep 16, 2024
Merged
feat: aggregator should check if gas is enough before respondToTask#918
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Conversation
@uri-99
@uri-99uri-99
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR
  • Aggregator runs a series of new checks before submitting RespondToTask tx:

    • runs a simulation of RespondToTask and checks RespondToTaskFeeLimit is enough
    • checks Batcher has enough funds to pay for RespondToTaskFeeLimit
    • checks Agg has enough funds to pay for RespondToTaskFeeLimit
In this PR, the variable batchersBalances was changed from internal to public; because the aggregator must check how much balance does the Batcher has, to compare it with RespondToTaskFeeLimit.
To Deploy
Aggregator code has changed (adding new checks) , so it must be redeployed.

AlignedLayerServiceManager variable was changed from internal to public, so it must be redeployed.
To Test
Happy Path
make anvil_start_with_block_time

make aggregator_start

make operator_register_and_start

make batcher_start_local

make run_explorer

And when they finish starting up, you can send proofs:
make batcher_send_risc0_burst

Unhappy paths
  • Empty Aggregator balance:
cast send 0x0000000000000000000000000000000000000000 --value 9999998499999944500000 --private-key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a --rpc-url "http://localhost:8545"

Note cast may not show the tx as completed, but you can verify batchers balance with:
cast balance 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 --rpc-url localhost:8545

Then send proofs, Aggregator should not try to submit the tx
make batcher_send_risc0_burst

  • change the DEFAULT_AGGREGATOR_FEE_MULTIPLIER and RESPOND_TO_TASK_FEE_LIMIT_MULTIPLIER to different values, and check it behaves accordingly
  • hardcode respondToTaskFeeLimit values on the BatcherPaymentService or AlignedServiceManagerand, and check it behaves accordingly
  • (These 2 cases are pretty similar but one may be favourable over the other depending which one you understand better and which value you want to test more in detail)
taturosatiand others
added 30 commits
August 26, 2024 14:35
@taturosati
feat (batcher-payment-service): allow user to specify max fee
1a02d7d
@taturosati
feat (batcher-sdk): allow uesr to specify max fee
e7882fc
@taturosati
refactor: change & specify default max fee
5f4f248
@taturosati
refactor (batcher): use priority queue for batch queue
9fa7610
@taturosati
feat (batcher): check max fees when building batch
89243e2
@taturosati
feat (batcher): check max fee is less for bigger nonce
32de669
@taturosati
refactor (batcher): use priority queue lib
f309627
@taturosati
feat (batcher): allow replacement higher fee
10cb529
@taturosati
chore: merge
3d01edd
@taturosati
fix: solhint
6fadf42
@taturosati
fix: handle cache reset correctly
08c7c89
@taturosati
fix (batcher): handling of replacement fee
d991b62
@taturosati
refactor: extract gas check multiplier to constant
f62da9f
@taturosati
chore: merge
03bd4e6
@taturosati
fix: dont allow size 1 batch
1983185
@taturosati
fix: update ws connection on replacemnt tx
f20679a
@taturosati
docs: max_fee
a81a870
@taturosati
fix: verify max fee
0620d83
@taturosati
chore: add comment on user_min_fee
8e4303a
taturosati
refactor: check nonce and max fee
82cb11f
taturosati
refactor: is_batch_ready
92090ab
@uri-99
chore: add error signature on new error
d94cd6d
taturosati
chore: remove old comment
cc382cd
taturosati
feat: close old sink
6a9b7ac
taturosati
chore: update function docs
eaa9e78
taturosati
chore: merge
1d83e63
taturosati
fix (aligned-sdk): add batcher address to check is batch verifed
70a1482
taturosati
refactor: rename batcher_addr to payment_service_addr on aligned cli
5cca7ea
taturosati
docs: add batcher address to sdk methods
29cfbd0
taturosati
refactor: apply rename suggestions from code review
fb9934d

Base automatically changed from
881-feat-add-limit-on-aggregator-spending to
mainSeptember 6, 2024 18:41
NicolasRampoldi
NicolasRampoldi
suggested changes
Sep 10, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Outdated

Comment threadcore/chainio/avs_writer.go
Outdated

taturosati
taturosati
reviewed
Sep 11, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Comment threadcore/chainio/avs_writer.go
Outdated

@uri-99uri-99
closed this
Sep 16, 2024
@uri-99uri-99
reopened this
Sep 16, 2024
@github-actions
github-actionsBot
commented
Sep 16, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: 2c0465a1da5fb2e15359522e421bbcf8d12b5cce, compared to commit: 4fe73dae65b9341ad33a212586564a2a8da8ea3a
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
AlignedLayerServiceManagerbatchesState-22 ✅-3.03%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,660,098 (+12,081)batchesState
createNewTask
receive		703 (-22)
56,921 (+2)
21,169 (0)		-3.03%
+0.00%
0.00%		703 (-22)
76,896 (+76)
44,783 (+93)		-3.03%
+0.10%
+0.21%		703 (-22)
76,977 (-46)
45,064 (0)		-3.03%
-0.06%
0.00%		703 (-22)
78,130 (+17)
45,064 (0)		-3.03%
+0.02%
0.00%		256 (0)
256 (0)
256 (0)
@entropidelic
entropidelic
merged commit 047a65b
into
stagingSep 16, 2024
@entropidelic
entropidelic
deleted the

882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask

branch
September 16, 2024 22:16
@uri-99uri-99
mentioned this pull request
Sep 23, 2024

Closed

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

Reviewers

@NicolasRampoldiNicolasRampoldiAwaiting requested review from NicolasRampoldi

3 more reviewers

@taturosatitaturosatitaturosati left review comments
@PatStilesPatStilesPatStiles approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@uri-99uri-99

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

feat: Aggregator should check if gas is enough before respondToTask


6 participants

@uri-99@PatStiles@NicolasRampoldi@taturosati@entropidelic@glpecile

, '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); }
})();
})();
feat: aggregator should check if gas is enough before respondToTask by uri-99 · Pull Request #918 · yetanotherco/aligned_layer · GitHub
Skip to content
feat: aggregator should check if gas is enough before respondToTask - #918
Merged
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Sep 16, 2024
Merged
feat: aggregator should check if gas is enough before respondToTask#918
entropidelic merged 131 commits into
stagingfrom 
882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask
Conversation
@uri-99
@uri-99uri-99
commented
Sep 3, 2024•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR
  • Aggregator runs a series of new checks before submitting RespondToTask tx:

    • runs a simulation of RespondToTask and checks RespondToTaskFeeLimit is enough
    • checks Batcher has enough funds to pay for RespondToTaskFeeLimit
    • checks Agg has enough funds to pay for RespondToTaskFeeLimit
In this PR, the variable batchersBalances was changed from internal to public; because the aggregator must check how much balance does the Batcher has, to compare it with RespondToTaskFeeLimit.
To Deploy
Aggregator code has changed (adding new checks) , so it must be redeployed.

AlignedLayerServiceManager variable was changed from internal to public, so it must be redeployed.
To Test
Happy Path
make anvil_start_with_block_time

make aggregator_start

make operator_register_and_start

make batcher_start_local

make run_explorer

And when they finish starting up, you can send proofs:
make batcher_send_risc0_burst

Unhappy paths
  • Empty Aggregator balance:
cast send 0x0000000000000000000000000000000000000000 --value 9999998499999944500000 --private-key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a --rpc-url "http://localhost:8545"

Note cast may not show the tx as completed, but you can verify batchers balance with:
cast balance 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 --rpc-url localhost:8545

Then send proofs, Aggregator should not try to submit the tx
make batcher_send_risc0_burst

  • change the DEFAULT_AGGREGATOR_FEE_MULTIPLIER and RESPOND_TO_TASK_FEE_LIMIT_MULTIPLIER to different values, and check it behaves accordingly
  • hardcode respondToTaskFeeLimit values on the BatcherPaymentService or AlignedServiceManagerand, and check it behaves accordingly
  • (These 2 cases are pretty similar but one may be favourable over the other depending which one you understand better and which value you want to test more in detail)
taturosatiand others
added 30 commits
August 26, 2024 14:35
@taturosati
feat (batcher-payment-service): allow user to specify max fee
1a02d7d
@taturosati
feat (batcher-sdk): allow uesr to specify max fee
e7882fc
@taturosati
refactor: change & specify default max fee
5f4f248
@taturosati
refactor (batcher): use priority queue for batch queue
9fa7610
@taturosati
feat (batcher): check max fees when building batch
89243e2
@taturosati
feat (batcher): check max fee is less for bigger nonce
32de669
@taturosati
refactor (batcher): use priority queue lib
f309627
@taturosati
feat (batcher): allow replacement higher fee
10cb529
@taturosati
chore: merge
3d01edd
@taturosati
fix: solhint
6fadf42
@taturosati
fix: handle cache reset correctly
08c7c89
@taturosati
fix (batcher): handling of replacement fee
d991b62
@taturosati
refactor: extract gas check multiplier to constant
f62da9f
@taturosati
chore: merge
03bd4e6
@taturosati
fix: dont allow size 1 batch
1983185
@taturosati
fix: update ws connection on replacemnt tx
f20679a
@taturosati
docs: max_fee
a81a870
@taturosati
fix: verify max fee
0620d83
@taturosati
chore: add comment on user_min_fee
8e4303a
taturosati
refactor: check nonce and max fee
82cb11f
taturosati
refactor: is_batch_ready
92090ab
@uri-99
chore: add error signature on new error
d94cd6d
taturosati
chore: remove old comment
cc382cd
taturosati
feat: close old sink
6a9b7ac
taturosati
chore: update function docs
eaa9e78
taturosati
chore: merge
1d83e63
taturosati
fix (aligned-sdk): add batcher address to check is batch verifed
70a1482
taturosati
refactor: rename batcher_addr to payment_service_addr on aligned cli
5cca7ea
taturosati
docs: add batcher address to sdk methods
29cfbd0
taturosati
refactor: apply rename suggestions from code review
fb9934d

Base automatically changed from
881-feat-add-limit-on-aggregator-spending to
mainSeptember 6, 2024 18:41
NicolasRampoldi
NicolasRampoldi
suggested changes
Sep 10, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Outdated

Comment threadcore/chainio/avs_writer.go
Outdated

taturosati
taturosati
reviewed
Sep 11, 2024
View reviewed changes
Comment threadcore/chainio/avs_writer.go
Comment threadcore/chainio/avs_writer.go
Outdated

@uri-99uri-99
closed this
Sep 16, 2024
@uri-99uri-99
reopened this
Sep 16, 2024
@github-actions
github-actionsBot
commented
Sep 16, 2024•

edited

Loading

Copy link


Copy Markdown

Changes to gas cost
Generated at commit: 2c0465a1da5fb2e15359522e421bbcf8d12b5cce, compared to commit: 4fe73dae65b9341ad33a212586564a2a8da8ea3a
🧾 Summary (10% most significant diffs)
ContractMethodAvg (+/-)%
AlignedLayerServiceManagerbatchesState-22 ✅-3.03%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,660,098 (+12,081)batchesState
createNewTask
receive		703 (-22)
56,921 (+2)
21,169 (0)		-3.03%
+0.00%
0.00%		703 (-22)
76,896 (+76)
44,783 (+93)		-3.03%
+0.10%
+0.21%		703 (-22)
76,977 (-46)
45,064 (0)		-3.03%
-0.06%
0.00%		703 (-22)
78,130 (+17)
45,064 (0)		-3.03%
+0.02%
0.00%		256 (0)
256 (0)
256 (0)
@entropidelic
entropidelic
merged commit 047a65b
into
stagingSep 16, 2024
@entropidelic
entropidelic
deleted the

882-feat-aggregator-should-check-if-gas-is-enough-before-respondtotask

branch
September 16, 2024 22:16
@uri-99uri-99
mentioned this pull request
Sep 23, 2024

Closed

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

Reviewers

@NicolasRampoldiNicolasRampoldiAwaiting requested review from NicolasRampoldi

3 more reviewers

@taturosatitaturosatitaturosati left review comments
@PatStilesPatStilesPatStiles approved these changes
@entropidelicentropidelicentropidelic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@uri-99uri-99

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

feat: Aggregator should check if gas is enough before respondToTask


6 participants

@uri-99@PatStiles@NicolasRampoldi@taturosati@entropidelic@glpecile