fix: make is_merchant_registered TTL-neutral (#563) - #648
Open
Aj-Kayvee wants to merge 10 commits into
Open
Conversation
Public read path in is_merchant_registered delegated to is_merchant_registered_internal, which extended the merchant entry's TTL on every call. This caused a read-only oracle query to mutate storage — an undesirable side effect. Decouple the two paths: - Add is_merchant_registered_read — a pure persistent.has() check with no TTL bump, used by the public entry point. - Keep is_merchant_registered_internal unchanged for internal callers (settlement, payments, admin migration) that intentionally warm the merchant marker. Also adds a regression test that verifies the public read does not increase the merchant entry's TTL.
|
@Aj-Kayvee 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! 🚀 |
🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
The merged main added a deployer-prefixed init to both contracts, but
governance_error_tests.rs kept calling init with the old argument counts,
referenced GovernanceContract/GovernanceContractClient via the wrong
crate root, and used Symbol without importing it (or FromVal). Translate
the tests to the current signatures and bring the governance types into
scope:
- Import governance_contract::{FeeConfig as GovFeeConfig,
GovernanceContract, GovernanceContractClient} instead of crate::.
- Add Symbol and FromVal to the soroban_sdk imports.
- Collapse the duplicated 4-argument + 5-argument init calls into a
single deployer-prefixed init.
- Pass a deployer to every settlement/governance init call site.
…branch Fix all test/build failures that kept CI red on this branch: - deduplicate storage imports in admin.rs and merchant.rs - fix Val/Symbol conversion and unused-import warnings in storage.rs - pass deployer to client init/try_init calls across both contracts - pass executor to execute calls and align permissionless execute policy with the documented uniform execution auth (drop executor.require_auth) - add missing deployer declarations in admin_tests, governance_error, anchor_no_event_error and real_auth tests - update stale tests to current intended behavior: network-fee clamp (Betta-Pay#683), tombstone cleared on re-registration (Betta-Pay#685), and no bootstrap_fallback event on the hot path (Betta-Pay#689/Betta-Pay#691) - apply cargo fmt across the workspace All clippy warnings/errors cleared; full workspace test suite passes.
The Tests workflow never installs the soroban CLI, so 'make all' always failed at 'make wasm_size' (soroban: not found). Scope the trailing gate to the code checks that the workflow can actually run; the wasm-size gate remains enforced by the auto-merge workflow, which installs soroban-cli.
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
is_merchant_registered(public entry point) delegates tois_merchant_registered_internal, which extends the merchant entry's TTL on every call. This means a read-only oracle check mutates storage — an undesirable side effect that also exposes TTL manipulation through a public query.Problem
The public
is_merchant_registered(merchant.rs:87-92) guards onDataKey::Adminpresence and then callsis_merchant_registered_internal, which unconditionally runsextend_ttlon the merchant persistent entry when the merchant exists. This causes:bool-returning public getter shouldn't extend storage lifetime.Solution
Decouple the read path from the admin-warm path:
is_merchant_registered_read(storage.rs) — a purepersistent.has()check with no TTL bump. Used by the public entry point.is_merchant_registered_internal— unchanged. Continues to warm the merchant marker for internal callers (settlement,payments,adminmigration) that intentionally keep entries alive.Files changed
settlement_contract/src/storage.rsis_merchant_registered_read— TTL-neutralhas()checksettlement_contract/src/merchant.rs_internal→_readsettlement_contract/src/tests/admin_tests.rsis_merchant_registered_is_ttl_neutralregression testRegression test
is_merchant_registered_is_ttl_neutralregisters a merchant, advances the ledger, then asserts that calling the publicis_merchant_registereddoes not increase the merchant entry's TTL.Verification
Closes #563