Add is_max_settlement FIX#238
Open
Mitch5000 wants to merge 1 commit into
Open
Conversation
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.
Pull Request Title:
Fix max settlement amount configuration ambiguity
Pull Request Description:
Summary
This PR fixes an audit-trail ambiguity in the max settlement amount configuration.
Previously, max_settlement_amount(env, asset) returned 0 both when:
set_max_settlement_amount(asset, 0) was explicitly called by an admin
set_max_settlement_amount was never called for that asset
Because 0 disables the cap, those two states were indistinguishable on-chain.
This PR adds a configuration-status accessor so clients and auditors can distinguish “never configured” from “explicitly configured to zero”, without changing the actual open_settlement cap enforcement behavior.
Changes Made
Added storage helper
File:
src/storage.rs
Added:
has_max_settlement_amount(env, asset) -> bool
Behavior:
Returns false when MaxSettlementAmount(asset) has never been written.
Returns true after set_max_settlement_amount has been called for the asset.
Returns true even when the configured value is 0.
Extends TTL when the entry exists, matching the existing max settlement amount read behavior.
Added public contract read view
File:
src/lib.rs
Added on-chain contract function:
is_max_settlement_amt_configured(env, asset) -> bool
Note:
The originally requested name, is_max_settlement_amount_configured, exceeds Soroban’s 32-byte contract function name limit.
To satisfy Soroban’s function symbol limit, the exported on-chain function uses amt instead of amount.
Also added a Rust client convenience alias:
is_max_settlement_amount_configured(asset) -> bool
This preserves the issue terminology for Rust tests and client-side usage while keeping the actual exported contract function valid.
Preserved open_settlement behavior
File:
src/lib.rs
The cap enforcement logic remains unchanged:
cap == 0 means disabled
cap > 0 enforces the maximum settlement amount
No settlement enforcement behavior was changed.
Added regression tests
File:
src/test.rs
Added and extended tests covering:
Never-configured max settlement amount returns:
max_settlement_amount(asset) == 0
is_max_settlement_amount_configured(asset) == false
Explicit zero configuration returns:
max_settlement_amount(asset) == 0
is_max_settlement_amount_configured(asset) == true
open_settlement still allows settlement because zero disables the cap
Nonzero configuration returns:
max_settlement_amount(asset) == configured value
is_max_settlement_amount_configured(asset) == true
Safe read behavior for an unconfigured asset.
Updated documentation
File:
README.md
Documented the new max settlement configuration-status view.
Dev dependency update
File:
Cargo.toml
Cargo.lock
Enabled the Soroban SDK testutils feature for dev-dependencies so the existing test suite compiles and runs correctly.
Soroban function-name compatibility fix
File:
src/lib.rs
README.md
While validating the build, Soroban rejected an existing public function name because it exceeded the 32-byte contract function name limit:
list_settlements_by_anchor_and_asset
This was shortened on-chain to:
list_settlements_by_anchor_asset
A Rust client compatibility alias was added so existing Rust tests and client code can still call:
list_settlements_by_anchor_and_asset
Validation
Ran formatting check:
cargo fmt --all -- --check
Result:
Passed
Ran full test suite:
CARGO_BUILD_JOBS=1 cargo test
Result:
239 passed
0 failed
Ran build:
cargo build
Result:
Passed
Notes
The build still emits existing Soroban deprecation warnings related to env.events().publish usage. These warnings are unrelated to this fix and were not introduced by this PR.
Files Modified
Cargo.toml
Cargo.lock
README.md
src/storage.rs
src/lib.rs
src/test.rs
Files Created
None
Confidence
96%
The fix directly addresses the described ambiguity, preserves settlement cap behavior, adds regression coverage for all required states, and passes the full test suite and build.
Closes #109