chore: review storage footprint and compute-cost of hot paths (#144-#147) - #319
Open
maztah1 wants to merge 1 commit into
Open
Conversation
Field-by-field review of IntentRecord/SolverRecord, compute_intent_id's preimage build, unconditional extend_ttl calls, and the single instance-storage entry. Each is documented under docs/ with the finding and recommendation; none warrants a code change. Closes stellar-vortex-protocol#147 Closes stellar-vortex-protocol#146 Closes stellar-vortex-protocol#145 Closes stellar-vortex-protocol#144
|
@maztah1 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four related performance/footprint investigations against
intent_settlement. Each issue asked for a review first and a code change only if net-positive; in all four cases the finding is that no code change is warranted, and each is documented underdocs/so the reasoning is on record.IntentRecord/SolverRecordfield types & ordering#[contracttype]structs serialize as a key-sortedScMap, so field ordering has zero footprint effect. Every integer field is already at the correct width —u64timestamps are SDK-native, and thei128amount fields are bounded byMAX_AMOUNT(1e30 > u64::MAX) so they genuinely need 128 bits (u128saves 0 bytes). The only real wins are structural (SolverRecord.addressduplicates its storage key;fill_amountvstotal_filledlook redundant) and belong in their own issues.compute_intent_idfixed-size preimage bufferAddress/StringXDR), andto_xdrmust cross the host boundary regardless, so a fixed-size buffer doesn't cleanly apply. The only collapsible part — packing the 3 trailing integers into one append — removes ~2 of ~8 host calls, a sub-1% effect dominated bysha256+to_xdr.extend_ttlon every writeextend_ttl(threshold, extend_to)is already conditional inside the host: below-threshold TTL is a subtract-and-compare with no ledger write and no rent. A guest-side pre-check would callget_ttl(another host call) and only add cost.docs/ttl-constants-rationale.md), so every instance write rewrites the whole map — but that map is a dozen small scalars. MovingTotalIntents/TotalVolumeto their own persistent entries adds a full extra ledger read + write + TTL/rent lifecycle per call — strictly more I/O, not less. The counter update is already dwarfed by the persistentIntentRecordwrite everysubmit_intentdoes.Notes
docs/149-resource-cost-per-entrypoint.md); the docs record what can be measured once it lands and the conditions under which each change would become worth revisiting.Changes
docs/147-intentrecord-solverrecord-field-footprint-review.mddocs/146-compute-intent-id-preimage-buffer-benchmark.mddocs/145-ttl-bump-frequency-review.mddocs/144-instance-storage-layout-review.mdCloses #147
Closes #146
Closes #145
Closes #144