Bump default resource limits in NewPaymentToContract for protocol 26 - #5921
Conversation
The previous defaults (Instructions: 400K, DiskReadBytes: 1K, WriteBytes: 1K, ResourceFee: 5M) are too low for protocol 26, causing resource_limit_exceeded errors. Bump to values that accommodate the increased resource costs. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
| DiskReadBytes: 1_000, | ||
| WriteBytes: 1_000, | ||
| ResourceFee: 5_000_000, | ||
| Instructions: 5_000_000, |
There was a problem hiding this comment.
can we reference the slack thread or a ticket that details why this was needed for contextual reference
There was a problem hiding this comment.
yeah it'd be good to get a "why" here (or bisect a better lower bound) because 10x on resources is going to make these invocations a lot more expensive (only the ResourceFee is refundable, the resources themselves get allocated [and thus paid for] regardless)
There was a problem hiding this comment.
These defaults are used by integration tests that exercise SAC transfers. On protocol 26, the old Instructions limit (400K) is too low — transactions fail with resource_limit_exceeded. This is due to increased instruction costs in the protocol 26 Soroban host.
Ran the SAC transfer through RPC simulation on protocol 26 to get the actual resource requirements:
| Resource | Simulated | Old Default | This PR |
|---|---|---|---|
| Instructions | 702,564 | 400,000 | 5,000,000 |
| DiskReadBytes | 144 | 1,000 | 10,000 |
| WriteBytes | 224 | 1,000 | 10,000 |
| ResourceFee | 216,925 | 5,000,000 | 10,000,000 |
Only Instructions exceeds the old default (703K vs 400K). The other three values are well within the old limits.
Verified that these tighter defaults pass both the first-mint and existing-balance SAC transfer paths on protocol 26:
var defaultPaymentToContractFees = SorobanFees{
Instructions: 2_000_000,
DiskReadBytes: 1_000,
WriteBytes: 1_000,
ResourceFee: 5_000_000,
}Could you take another look with these findings in mind?
There was a problem hiding this comment.
These defaults are used by integration tests that exercise SAC transfers.
They're also a part of the whole "let people make simple payments without simulation" SDK idea, hence why I brought up real costs for people if they're using these. We should only bump what we need, i.e. undo the bump everywhere except the instruction case, so I agree w/ the tighter bounds you posted, but I don't see a commit undo'ing the bump.
RPC simulation of a SAC transfer on protocol 26 shows: - Instructions: 702,564 (actual) → 2,000,000 (~3x headroom) - DiskReadBytes: 144 (actual) → 1,000 (old default sufficient) - WriteBytes: 224 (actual) → 1,000 (old default sufficient) - ResourceFee: 216,925 (actual) → 5,000,000 (old default sufficient) Only Instructions needed to increase from the old defaults. The previous 10x bump across all values was unnecessarily aggressive. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
* Update XDR definitions to cff714a5ebaaaf2dac343b3546c2df73f0b7a36e (#5919) * Bump default resource limits in NewPaymentToContract for protocol 26 (#5921) --------- Co-authored-by: tamirms <[email protected]>
4 participants
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
CHANGELOG.mdwithin the component folder structure. For example, if I changed horizon, then I updated (services/horizon/CHANGELOG.md. I add a new line item describing the change and reference to this PR. If I don't update a CHANGELOG, I acknowledge this PR's change may not be mentioned in future release notes.semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
The previous defaults (Instructions: 400K, DiskReadBytes: 1K, WriteBytes: 1K, ResourceFee: 5M) are too low for protocol 26, causing resource_limit_exceeded errors. Bump to values that accommodate the increased resource costs.