Skip to content
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.

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

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

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from

Conversation

uri-99
Copy link
Contributor

@uri-99 uri-99 commented Jan 20, 2025

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
Copy link

github-actions bot commented Feb 10, 2025

Changes to gas cost

Generated at commit: d5155ba7de1afc49012194d8e889d6da81ada265, compared to commit: 306f72d2c0f03f0107f55725c2ba5fa168f39fc4

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
RegistryCoordinatorHarness blsApkRegistry +332 ❌ +116.90%
Slasher initialize +470 ❌ +103.98%
StakeRegistryHarness delegation +332 ❌ +101.84%
AlignedLayerServiceManager batchesState +688 ❌ +95.29%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
RegistryCoordinatorHarness 10,130,048 (+4,267,886) 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,813 (+762,834) 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)
AlignedLayerServiceManager 8,615,423 (+3,384,189) batchesState
createNewTask
disableVerifier
disabledVerifiers
enableVerifier
isVerifierDisabled
receive
setDisabledVerifiers
1,410 (+688)
57,806 (+1,723)
24,299 (+390)
514 (+131)
24,191 (+390)
856 (+275)
23,496 (+179)
24,250 (+451)
+95.29%
+3.07%
+1.63%
+34.20%
+1.64%
+47.33%
+0.77%
+1.90%
1,410 (+688)
77,949 (+1,888)
35,816 (+315)
514 (+131)
24,760 (+315)
1,522 (+275)
47,378 (+463)
35,176 (+370)
+95.29%
+2.48%
+0.89%
+34.20%
+1.29%
+22.05%
+0.99%
+1.06%
1,410 (+688)
77,946 (+1,705)
35,816 (+315)
514 (+131)
24,760 (+315)
856 (+275)
47,472 (+277)
35,176 (+370)
+95.29%
+2.24%
+0.89%
+34.20%
+1.29%
+47.33%
+0.59%
+1.06%
1,410 (+688)
78,705 (+1,711)
47,334 (+240)
514 (+131)
25,329 (+240)
2,856 (+275)
47,472 (+277)
46,103 (+290)
+95.29%
+2.22%
+0.51%
+34.20%
+0.96%
+10.65%
+0.59%
+0.63%
256 (0)
256 (0)
2 (0)
1 (0)
2 (0)
3 (0)
256 (0)
2 (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,771 (+1,054,437) 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,545 (+637,885) 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)

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants