Skip to content

feat(contract): withdraw_from_service_manager in BatcherPaymentService.sol #1772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
JuArce merged 9 commits into from
Mar 19, 2025
Merged

feat(contract): withdraw_from_service_manager in BatcherPaymentService.sol #1772

JuArce merged 9 commits into from
Mar 19, 2025

Conversation

@uri-99
Copy link
Contributor

@uri-99 uri-99 commented Jan 20, 2025 •
edited by JuArce
Loading

withdraw_from_service_manager in BatcherPaymentService.sol

Description

AlignedLayerServiceManager has a withdraw() function, which is currently not accesible to the Batcher, since he pays through the BatcherPaymentService.

With this function, the Batcher will be able to withdraw funds from AlignedLayerServiceManager.

How to Test

Addresses:

"alignedLayerServiceManager": "0x851356ae760d987E095750cCeb3bC6014560891C",
"batcherPaymentService": "0x7bc06c482DEAd17c0e297aFbC32f6e63d3846650",
  1. To test this feature, you just need to start Anvil
make anvil_start_with_block_time
  1. Check balanceOf batcherPaymentService on AlignedLayerServiceManager
cast call 0x851356ae760d987E095750cCeb3bC6014560891C "balanceOf(address) (uint256)" 0x7bc06c482DEAd17c0e297aFbC32f6e63d3846650 --rpc-url http:localhost:8545

Note: The anvil deployment script already funds the Batcher balance with 1ETH

The output should be

1000000000000000000 [1e18]
  1. Withdraw Batcher funds to an arbitrary wallet (0x0a680FeAE94ffE26EcC8262bf04CE64C7b495cfD)

The batcherPaymentService owner is 0x14dC79964da2C08b23698B3D3cc7Ca32193d9955, and it private key is 0x4bbbf85ce3377467afe5d46f804f221813b2bb87f24d81f60f1fcdbf7cbf4356 (Address 7 in anvil)

cast send 0x7bc06c482DEAd17c0e297aFbC32f6e63d3846650 "withdrawFromServiceManager(uint256 amount, address withdrawAddress)" 500000000000000000 0x0a680FeAE94ffE26EcC8262bf04CE64C7b495cfD --rpc-url http:localhost:8545 --private-key 0x4bbbf85ce3377467afe5d46f804f221813b2bb87f24d81f60f1fcdbf7cbf4356
  1. Check the batcher balance in the AlignedLayerServiceManager
cast call 0x851356ae760d987E095750cCeb3bC6014560891C "balanceOf(address) (uint256)" 0x7bc06c482DEAd17c0e297aFbC32f6e63d3846650 --rpc-url http:localhost:8545

The output should be

500000000000000000 [5e17]
  1. Check the balance of the arbitrary wallet who received the funds (0x0a680FeAE94ffE26EcC8262bf04CE64C7b495cfD)
cast balance --rpc-url http://localhost:8545 0x0a680FeAE94ffE26EcC8262bf04CE64C7b495cfD --ether

The output should be

0.5

Type of change

Please delete options that are not relevant.

  • New feature

Checklist

  • “Hotfix” to testnet, everything else to staging
  • Linked to Github Issue
  • This change depends on code or research by an external entity
    • Acknowledgements were updated to give credit
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run
  • Has a known issue
  • If your PR changes the Operator compatibility (Ex: Upgrade prover versions)
    • This PR adds compatibility for operator for both versions and do not change batcher/docs/examples
    • This PR updates batcher and docs/examples to the newer version. This requires the operator are already updated to be compatible

@uri-99 uri-99 self-assigned this Jan 20, 2025
@uri-99 uri-99 marked this pull request as draft January 20, 2025 21:27
@uri-99 uri-99 marked this pull request as ready for review January 28, 2025 19:40
@github-actions
Copy link

github-actions bot commented Feb 10, 2025 •
edited
Loading

Changes to gas cost

Generated at commit: 7012436c3ed8c01ba15ecd4a55c562af4a0d9ec5, compared to commit: 306f72d2c0f03f0107f55725c2ba5fa168f39fc4

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
AlignedLayerServiceManager batchesState
disabledVerifiers
isVerifierDisabled		 +4,710 ❌
+2,153 ❌
+1,609 ❌		 +652.35%
+562.14%
+129.03%
RegistryCoordinatorHarness blsApkRegistry +332 ❌ +116.90%
Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
AlignedLayerServiceManager 8,464,710 (+3,233,476) batchesState
createNewTask
disableVerifier
disabledVerifiers
enableVerifier
isVerifierDisabled
receive
setDisabledVerifiers		 5,432 (+4,710)
57,768 (+1,685)
24,299 (+390)
2,536 (+2,153)
24,191 (+390)
2,856 (+2,275)
23,496 (+179)
24,272 (+473)		 +652.35%
+3.00%
+1.63%
+562.14%
+1.64%
+391.57%
+0.77%
+1.99%		 5,432 (+4,710)
77,742 (+1,681)
35,816 (+315)
2,536 (+2,153)
24,760 (+315)
2,856 (+1,609)
47,284 (+369)
35,198 (+392)		 +652.35%
+2.21%
+0.89%
+562.14%
+1.29%
+129.03%
+0.79%
+1.13%		 5,432 (+4,710)
77,812 (+1,571)
35,816 (+315)
2,536 (+2,153)
24,760 (+315)
2,856 (+2,275)
47,472 (+277)
35,198 (+392)		 +652.35%
+2.06%
+0.89%
+562.14%
+1.29%
+391.57%
+0.59%
+1.13%		 5,432 (+4,710)
78,586 (+1,592)
47,334 (+240)
2,536 (+2,153)
25,329 (+240)
2,856 (+275)
47,472 (+277)
46,125 (+312)		 +652.35%
+2.07%
+0.51%
+562.14%
+0.96%
+10.65%
+0.59%
+0.68%		 256 (0)
256 (0)
2 (0)
1 (0)
2 (0)
3 (0)
256 (0)
2 (0)
RegistryCoordinatorHarness 10,130,060 (+4,267,898) blsApkRegistry
initialize
stakeRegistry		 616 (+332)
56,281,475 (+1,577,892)
682 (+332)		 +116.90%
+2.88%
+94.86%		 616 (+332)
56,281,475 (+1,577,892)
682 (+332)		 +116.90%
+2.88%
+94.86%		 616 (+332)
56,281,475 (+1,577,892)
682 (+332)		 +116.90%
+2.88%
+94.86%		 616 (+332)
56,281,475 (+1,577,892)
682 (+332)		 +116.90%
+2.88%
+94.86%		 6 (0)
6 (0)
6 (0)
Slasher 1,598,825 (+762,846) initialize 922 (+470) +103.98% 922 (+470) +103.98% 922 (+470) +103.98% 922 (+470) +103.98% 6 (0)
StakeRegistryHarness 5,139,510 (+2,005,505) delegation
initializeQuorum		 658 (+332)
146,200 (+3,117)		 +101.84%
+2.18%		 658 (+332)
165,996 (+3,117)		 +101.84%
+1.91%		 658 (+332)
166,100 (+3,117)		 +101.84%
+1.91%		 658 (+332)
166,100 (+3,117)		 +101.84%
+1.91%		 6 (0)
1,152 (0)
ProxyAdmin 757,646 (+329,348) upgrade
upgradeAndCall		 39,641 (+845)
56,632,018 (+1,579,612)		 +2.18%
+2.87%		 39,650 (+845)
56,632,018 (+1,579,612)		 +2.18%
+2.87%		 39,653 (+845)
56,632,018 (+1,579,612)		 +2.18%
+2.87%		 39,653 (+845)
56,632,018 (+1,579,612)		 +2.18%
+2.87%		 24 (0)
6 (0)
AVSDirectory 3,174,782 (+1,463,555) initialize 100,637 (+2,514) +2.56% 100,637 (+2,514) +2.56% 100,637 (+2,514) +2.56% 100,637 (+2,514) +2.56% 6 (0)
ServiceManagerMock 2,632,783 (+1,054,449) initialize 73,362 (+1,409) +1.96% 73,362 (+1,409) +1.96% 73,362 (+1,409) +1.96% 73,362 (+1,409) +1.96% 6 (0)
BLSApkRegistryHarness 3,124,237 (+1,317,862) initializeQuorum
setBLSPublicKey		 45,978 (+772)
90,107 (+740)		 +1.71%
+0.83%		 45,978 (+772)
90,107 (+740)		 +1.71%
+0.83%		 45,978 (+772)
90,107 (+740)		 +1.71%
+0.83%		 45,978 (+772)
90,107 (+740)		 +1.71%
+0.83%		 1,152 (0)
6 (0)
TransparentUpgradeableProxy 897,999 (+341,241) fallback 1,544 (+477) +44.70% 48,066 (+774) +1.64% 8,110 (+477) +6.25% 120,000 (+885) +0.74% 30 (0)
IndexRegistry 1,707,557 (+637,897) initializeQuorum 45,592 (+528) +1.17% 45,592 (+528) +1.17% 45,592 (+528) +1.17% 45,592 (+528) +1.17% 1,152 (0)
StrategyManagerMock 2,401,372 (+1,154,891) setAddresses 89,742 (+597) +0.67% 89,742 (+597) +0.67% 89,742 (+597) +0.67% 89,742 (+597) +0.67% 6 (0)

JuArce
JuArce reviewed Feb 10, 2025
View reviewed changes
contracts/src/core/BatcherPaymentService.sol Outdated
userData[msg.sender].balance += msg.value;
userData[msg.sender].unlockBlockTime = 0;
emit PaymentReceived(msg.sender, msg.value);
if (msg.sender != address(alignedLayerServiceManager)) { // `alignedLayerServiceManager.withdraw()` triggers `receive()` (and with only 2300 gas)
Copy link
Collaborator

@JuArce JuArce Feb 10, 2025

Choose a reason for hiding this comment

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

@MauroToscano does it make sense to you?

Copy link
Contributor

@MauroToscano MauroToscano Feb 10, 2025

Choose a reason for hiding this comment

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

Uri mentioned it doesn't work without that, you can try just in case

@JuArce JuArce linked an issue Feb 18, 2025 that may be closed by this pull request
feat(contract): withdraw_from_service_manager in BatcherPayments.sol #1760
Closed
@JuArce JuArce enabled auto-merge March 19, 2025 18:01
@JuArce JuArce added this pull request to the merge queue Mar 19, 2025
Merged via the queue into staging with commit e546a4f Mar 19, 2025
4 checks passed
@JuArce JuArce deleted the 1760-featcontract-withdraw_from_service_manager-in-batcherpaymentssol branch March 19, 2025 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MauroToscano MauroToscano MauroToscano approved these changes

@JuArce JuArce JuArce approved these changes

Assignees

@uri-99 uri-99

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(contract): withdraw_from_service_manager in BatcherPayments.sol

4 participants

@uri-99 @MauroToscano @JuArce