feat: sponsored bundles (#25)#675
Conversation
This adds sponsorship of transactions that would otherwise fail with a LackOfFundForMaxFee. We check the senders balance first to detect the lack of funds rather than wait on the revert and then have to resimulate. * [x] Run `make lint` * [x] Run `make test` * [x] Added tests (if applicable)
There was a problem hiding this comment.
Pull Request Overview
This PR introduces transaction sponsorship functionality that allows the builder to fund transactions that would otherwise fail due to insufficient funds (LackOfFundForMaxFee). The system detects underfunded transactions before execution and injects sponsor transfers to enable profitable transactions to succeed.
Key changes:
- Pre-execution balance checking to detect underfunded transactions
- Automatic sponsor transaction injection with safety buffers and limits
- Comprehensive test coverage for sponsored transaction scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| crates/rbuilder/src/building/order_commit.rs | Core sponsorship logic with balance checks, sponsor transaction creation, and execution flow |
| crates/rbuilder/src/building/mod.rs | Added coinbase_signer field to BlockBuildingContext for sponsorship transactions |
| crates/rbuilder/src/building/testing/bundle_tests/setup.rs | Made try_commit_order method public for testing |
| crates/rbuilder/src/building/testing/bundle_tests/mod.rs | Added comprehensive test for sponsored transaction functionality |
| use tracing::trace; | ||
|
|
||
| /// Maximum amount that can be sponsored in a single transaction (2 ETH) | ||
| const MAX_SPONSOR_AMOUNT: U256 = U256::from_limbs([2_000_000_000_000_000_000, 0, 0, 0]); // 2 * 10^18 wei = 2 ETH |
There was a problem hiding this comment.
Magic number should be replaced with a more maintainable constant. Consider using U256::from(2) * U256::from(10).pow(U256::from(18)) or a similar expression that makes the calculation clearer.
| const MAX_SPONSOR_AMOUNT: U256 = U256::from_limbs([2_000_000_000_000_000_000, 0, 0, 0]); // 2 * 10^18 wei = 2 ETH | |
| const MAX_SPONSOR_AMOUNT: U256 = U256::from(2) * U256::from(10).pow(U256::from(18)); // 2 * 10^18 wei = 2 ETH |
|
|
||
| /// Safety buffer for sponsorship transactions to account for gas price changes, | ||
| /// timing differences, and EVM precision issues (0.001 ETH) | ||
| const SPONSORSHIP_SAFETY_BUFFER: U256 = U256::from_limbs([1_000_000_000_000_000, 0, 0, 0]); // 0.001 ETH |
There was a problem hiding this comment.
Magic number should be replaced with a more maintainable constant. Consider using U256::from(10).pow(U256::from(15)) or a similar expression that makes the calculation clearer.
| const SPONSORSHIP_SAFETY_BUFFER: U256 = U256::from_limbs([1_000_000_000_000_000, 0, 0, 0]); // 0.001 ETH | |
| const SPONSORSHIP_SAFETY_BUFFER: U256 = U256::from(10).pow(U256::from(15)); // 0.001 ETH |
| sponsor_signer, | ||
| builder_nonce, | ||
| sender, | ||
| 21_000, |
There was a problem hiding this comment.
The gas limit value 21_000 is a magic number. Consider defining it as a named constant like SIMPLE_TRANSFER_GAS_LIMIT to make its purpose clear and improve maintainability.
| 21_000, | |
| SIMPLE_TRANSFER_GAS_LIMIT, |
| &mut self.local_ctx.cached_reads, | ||
| )?; | ||
|
|
||
| let effective_gas_price = tx_inner.max_fee_per_gas(); |
There was a problem hiding this comment.
Using max_fee_per_gas() for fund calculation may be incorrect. For EIP-1559 transactions, the actual gas price depends on the base fee and priority fee. Consider using the effective gas price calculation that accounts for min(max_fee_per_gas, base_fee + max_priority_fee_per_gas).
| let effective_gas_price = tx_inner.max_fee_per_gas(); | |
| // Calculate effective gas price according to EIP-1559 rules | |
| let effective_gas_price = match tx_inner.tx_type() { | |
| // EIP-1559 transaction (type 2) | |
| Some(2) => { | |
| let max_fee_per_gas = tx_inner.max_fee_per_gas(); | |
| let max_priority_fee_per_gas = tx_inner.max_priority_fee_per_gas(); | |
| let base_fee = self.ctx.block_env.basefee; | |
| let base_fee_plus_priority = base_fee.saturating_add(max_priority_fee_per_gas); | |
| std::cmp::min(max_fee_per_gas, base_fee_plus_priority) | |
| } | |
| // Legacy or EIP-2930 transaction | |
| _ => tx_inner.gas_price(), | |
| }; |
|
|
||
| let sender1 = NamedAddr::User(1); | ||
|
|
||
| let large_value_to_coinbase = 999_999_999_999_999_999_u64; // Just under 1 ETH but gas will push it over |
There was a problem hiding this comment.
Magic number should be replaced with a named constant or calculated value. Consider using a more explicit calculation like U256::from(10).pow(U256::from(18)) - U256::from(1) to make the intent clearer.
| let large_value_to_coinbase = 999_999_999_999_999_999_u64; // Just under 1 ETH but gas will push it over | |
| // Just under 1 ETH but gas will push it over | |
| let large_value_to_coinbase = U256::from(10).pow(U256::from(18)) - U256::from(1); |
| } | ||
|
|
||
| // Get the correct nonce accounting for previous sponsor transactions in this bundle | ||
| let builder_nonce = { |
There was a problem hiding this comment.
this branch is not needed, its enough to just use else version of this branch
There was a problem hiding this comment.
Assuming you mean the if statement on line 800... when coinbase_payment is set to true in the config, the coin bases signing capability will be removed (builder_signer will be None). The coinbase is needed to sign the prepayment transaction for the sponsorship.
There was a problem hiding this comment.
I just realized I misread this one.. You're referring to the builder nonce code branch. This is handling the case when we sponsor multiple transactions within a single bundle. So the flow should be:
- Builder's onchain nonce is 100
- First sponsored tx: uses nonce 100, updates nonces_updated to (builder, 100)
- Second sponsored tx: finds (builder, 100) in nonces_updated, uses 101
- Third sponsored tx: finds (builder, 101) in nonces_updated, uses 102
| .unwrap(); | ||
|
|
||
| // Validate nonce to prevent reuse | ||
| let current_state_nonce = self.state.nonce( |
There was a problem hiding this comment.
why do you check nonce here, code above should give you correct nonce
There was a problem hiding this comment.
This is just defensive programming to ensure if there are any race conditions or alterations introduced accidentally in the future, we catch them. Our fork has a lot more trace logging I added at various points that would actually log this if it ever happened. If it did, we'd miss the slot and also cause demotions of optimistic relay submissions, so it's a critical error to catch.
| insert.cumulative_blob_gas_used, | ||
| )?; | ||
|
|
||
| trace!( |
There was a problem hiding this comment.
all other logs in the builder use style like this
trace!(?tx_hash, success = ..., "Sponsor tx execution result")
| if shortfall > U256::ZERO { | ||
| // Check if shortfall exceeds the maximum sponsor amount | ||
| if total_sponsor_amount > MAX_SPONSOR_AMOUNT { | ||
| continue; |
There was a problem hiding this comment.
If I read it correctly all these exits from the sponsorship branch like continue here are related to loop over txs in the bundle which means that this bundle tx will simply be skipped? If so then its not correct.
There was a problem hiding this comment.
No thats not correct. We are prchecking if a tx needs sponsorship prior to simulation. So if it needs sponsorship, and qualifies for sponsorship (ie below our MAX_SPONSOR_AMOUNT and assuming builder balance can support it), it will then attempt to add a sponsorship tx prior to the transaction that would otherwise fail. When it hits continue here, we just skip trying to add a sponsorship tx, and importantly roll back the state. So we're just skipping the shortfall loop, and that loop only attempts to add a sponsorship tx, it doesn't affect the tx that requires the sponsorship.
There was a problem hiding this comment.
See next comment, yes this is a bug introduced from extracting code from our fork to this one.
|
|
||
| // Update refundable_profit based on ACTUAL transaction profits, not optimistic bundle profit | ||
| // Only add profit from the original transaction if it's refundable and successful | ||
| if bundle.is_tx_refundable(&tx_hash) |
There was a problem hiding this comment.
I wonder if all of these can be simplified dramatically. Currently it adds a new branch for committing sponsored txs while it should be possible to do it in the same way so things like coinbase profit and refundable profit are handled by the same code that its handled now. Otherwise its a bit hard to verify that its correct.
For example, (see comment about continue above) I think this approach can lead to part of the bundle being committed while we should revert that bundle instead.
In other words, better approach would be so the sponsorhip path hits let result = self.commit_tx(
There was a problem hiding this comment.
So there should be no issues with part of the bundle being committed, see the above response. If a sponsorship tx added prior to an underfunded tx doesn't cause a successful tx (along with profit), then it rolls back to the point before the sponsorship attempt and proceeds normally, meaning the underfunded transaction will fail, causing the bundle to fail unless its in reverting/dropping hashes.
So I just realized I am looking at the code on our fork, not on this fork. This code is a fork from develop branch and I copied things over to this without double checking or testing it. Our fork has diverted significantly and the branching is different. So yes, I think as is this will cause a problem.
The main goal with this approach was to prevent simulation overhead. If we wait until LackOfFundForGasLimit, then we'll have to create a sponsorship transactions before the tx, then retry the tx, and this can happen multiple times in a bundle. There are a lot of LackOfFundForGasLimit simulation errors and the vast majority of them are not profitable to sponsor, so when we tested it, it did significantly increase simulation load with almost no benefit.
We also try not to make too many architecturally significant changes, that way future merges are easier for us. Because of that, we tend to prefer more code duplication rather than more cross cutting changes, as long as it doesn't affect the feature or the performance. So if you guys feel like there's a better long term architecture here, that makes sense, and we'd probably merge that in and help support it moving forward.
ZanCorDX
left a comment
There was a problem hiding this comment.
The main function is a huge piece of code with some continues which are error prone.
It feels that this could be implemented nicer with a function call what would handle the sponsoring and trying to have a single point for the tx execution instead of a copy/paste.
100% agree. I think I only communicated this on the issues thread. This was in no way meant for you all to merge. Our fork is just too different (we forked almost a year ago). I just ported over some code to demonstrate how it's accomplished. Thats also why I just left values hard coded, etc. Just want to demonstrate the gotchas on the implementation. The primary being checking the tx shortfall before simulation, and the nonce handling given multiple possible sponsorships and the fact the signer is usually randomized during simulation which doesn't work with sponsorship. |
Thanks. I'll close it then and we'll come to it for reference when we implement it. |
📝 Summary
This adds sponsorship of transactions that would otherwise fail with a LackOfFundForMaxFee.
We check the senders balance first to detect the lack of funds rather than wait on the revert and then have to resimulate.
💡 Motivation and Context
Transactions can fail because they lack funds, but they generate mev and would be actually to execute if they did have funds to do so.
✅ I have completed the following steps:
make lintmake test