Skip to content

fix: remove no-op validate_address and document rationale#1

Open
GhostStylez wants to merge 131 commits into
mainfrom
fix/validate-address-no-op
Open

fix: remove no-op validate_address and document rationale#1
GhostStylez wants to merge 131 commits into
mainfrom
fix/validate-address-no-op

Conversation

@GhostStylez
Copy link
Copy Markdown
Owner

Summary

Closes the issue: validate_address was a no-op that always returned Ok(()) without performing any real check.

Root Cause

In Soroban SDK v21, the Address type is constructed and validated by the host environment before contract code ever executes. An invalid or zero address cannot be passed in — the host traps first. There is also no "zero address" concept in Stellar/Soroban, and no runtime API to distinguish account vs contract addresses at the SDK level.

Changes

  • Removed validate_address from src/validation.rs
  • Added a comment block explaining why no address validation is needed (so future contributors don't re-add it)
  • Removed all call sites in:
    • validate_initialize_request
    • validate_create_remittance_request
    • validate_confirm_payout_request
    • validate_cancel_remittance_request
    • validate_withdraw_fees_request
    • validate_admin_operation
    • batch settlement loop in lib.rs
  • Prefixed now-unused address parameters with _ to suppress compiler warnings
  • Removed the now-redundant test_validate_valid_address test

Acceptance Criteria

  • validate_address removed with documented rationale
  • All call sites updated
  • Existing tests still pass (CI)

manuelusman73-png and others added 30 commits March 27, 2026 03:21
…ror code uniqueness

- All ContractError variants now have unique discriminant values (1-48)
- Added unit test to verify error codes are sequential and non-overlapping
- Test ensures no duplicate repr(u32) values exist in the enum
- Prevents future regressions with compile-time validation
…ce.rs

- Implement proper corridor ID formatting using byte concatenation
- Format now produces unique IDs for each (from, to) pair (e.g., 'US-MX')
- Add unit tests covering 3 corridor combinations (US-MX, MX-US, GB-NG)
- Ensures per-corridor fee differentiation works correctly
- Add validate_address_not_contract() to reject contract's own address
- Enhance validate_address() with comprehensive documentation
- Add unit tests covering address validation scenarios
- Prevent self-transfers and invalid address patterns
- Ensure validation is called consistently before state-changing operations
…or component

- Add 8 unit tests covering loading, error, and success states
- Test anchor selection and callback triggering
- Test currency filtering functionality
- Test details panel display and verified badge rendering
- Add vitest and React Testing Library dependencies
- Create vitest configuration for frontend testing
…ing example

- Create self-contained example component demonstrating AnchorSelector usage
- Show all major props: onSelect, currency, apiUrl
- Include currency filtering dropdown
- Display selected anchor details with formatted information
- Add inline comments explaining each prop and usage pattern
- Include help text and action button for complete workflow demonstration
- Add comprehensive security audit status with severity ratings
- Document test coverage metrics (92% coverage, 210+ tests)
- Include performance analysis and gas efficiency metrics
- List mainnet deployment prerequisites and checklist
- Define monitoring and alerting setup requirements
- Document incident response plan and procedures
- Verify all acceptance criteria met
- Report ready for stakeholder review
…component

- Add pageSize prop (default: 10) for configurable items per page
- Add currentPage state with prev/next navigation buttons
- Display 'Showing X–Y of Z transactions' indicator
- Support both controlled and uncontrolled pagination modes
- Reset pagination when transactions prop changes
- Add comprehensive keyboard navigation with ARIA attributes
- Implement 12 unit tests covering all pagination scenarios
- Add CSS styling for pagination controls and info display
- Ensure accessibility with aria-live regions and proper labels
…ce creation

- Add IdempotencyRecord struct to store key, hash, remittance_id, and expiry
- Add IdempotencyConflict error (code 49) for payload mismatch detection
- Add idempotency storage functions: get/set_idempotency_record, get/set_idempotency_ttl
- Add compute_request_hash function for deterministic payload hashing
- Update create_remittance to accept optional idempotency_key parameter
- Implement idempotency check: return existing remittance_id on duplicate key
- Detect conflicts when same key used with different payload
- Store idempotency records with configurable TTL (default: 24 hours)
- Lazy expiry: expired records allow re-execution with same key
- Backward compatible: requests without keys work unchanged
- Add DataKey::IdempotencyRecord and DataKey::IdempotencyTTL to storage
…tion

- Complete design.md with comprehensive proof validation architecture
- Add ProofData struct for signed proof encapsulation
- Add SettlementConfig struct for per-settlement proof requirements
- Update Remittance struct to include optional settlement_config
- Add InvalidProof (50), MissingProof (51), InvalidOracleAddress (52) errors
- Create verification.rs module with verify_proof() function
- Implement Ed25519 signature validation using Stellar SDK
- Update create_remittance to accept and validate settlement_config
- Update confirm_payout to validate proof before settlement execution
- Enforce proof requirement: return MissingProof if required but not provided
- Detect invalid proofs: return InvalidProof if signature validation fails
- Maintain backward compatibility: settlements without proof config work unchanged
- Add unit tests for proof verification edge cases
- Support flexible proof validation: optional per settlement
…r#198-201

- Document all four features implemented
- Include acceptance criteria verification
- List all files changed and lines added
- Provide deployment checklist
- Confirm backward compatibility
- Ready for mainnet deployment
- Added `get_fee_breakdown` function to provide detailed fee breakdowns for transactions.
- Supports optional country parameters for corridor-specific fees.
- Implemented various fee strategies: Percentage, Flat, and Dynamic.
- Created comprehensive unit tests covering multiple scenarios, including edge cases and error handling.
- Added documentation and testing guide for the new function.
- Add `.github/workflows/contract-ci.yml` workflow that runs on push/PR to main
- Execute `cargo test` to catch contract regressions automatically
- Build optimized WASM binary with `cargo build --target wasm32-unknown-unknown --release`
- Report WASM binary size in workflow summary
- Cache cargo artifacts (registry, git, build) for faster CI runs
- Fail workflow if tests fail (no continue-on-error for test job)
- Add caching for both native and WASM builds

Closes Haroldwonder#227
Add 3 proptest-based property tests to test_fee_strategy.rs:

- prop_fee_never_negative: verifies platform_fee and net_amount are
  always >= 0 across Percentage, Flat, and Dynamic strategies for
  any valid amount (1..=10_000_000_000)

- prop_fee_breakdown_sums_to_amount: verifies net_amount + platform_fee
  + protocol_fee == amount for all valid fee_bps (0..=10000) and
  protocol_fee_bps (0..=200), and that FeeBreakdown::validate() passes

- prop_dynamic_fee_tiers_monotonically_non_increasing: verifies the
  effective per-unit fee rate is non-increasing across tiers
  (Tier1 >= Tier2 >= Tier3) for any base_bps value

Each test runs 200 cases via proptest.
- Remove validate_address() — always returned Ok(()), providing false
  confidence without performing any check
- Document in module-level comment why address validation is not needed:
  Soroban SDK guarantees structural validity at the host layer, there is
  no zero address in Stellar, and a blanket account-vs-contract check
  would break existing callers that intentionally pass contract addresses
- Remove all 8 call sites across validate_initialize_request,
  validate_create_remittance_request, validate_confirm_payout_request,
  validate_cancel_remittance_request, validate_withdraw_fees_request,
  validate_admin_operation, and batch_settle_with_netting in lib.rs
- Add inline comments at each former call site explaining the omission
- Remove test_validate_valid_address (nothing meaningful to assert)
Haroldwonder and others added 30 commits March 27, 2026 20:28
test(backend): add e2e integration tests for remittance lifecycle
…y-doc-comments

docs: add inline doc comments to DataKey enum in storage.rs (Haroldwonder#212)
…-selector-example

feat: add AnchorSelector mock example for standalone/Storybook rendering
The debug-log feature was enabled by default in production builds,
which increases contract size and may leak internal state via events.

Changes:
- Remove debug-log from default features in Cargo.toml
- Add documentation to deploy.sh and deploy.ps1 explaining how to
  enable debug logging for local testing

Closes Haroldwonder#223
…t-feature

fix: remove debug-log from default features
…st-functions-238

feat: add admin blacklist management functions
…te-machine-docs-225

fix: reconcile remittance state machine with documentation
…npause-237

feat: add admin pause and unpause functions
…hors-230

feat: replace mock anchor data with database-backed storage
feat: update_check_in_interval Function
The validate_address function always returned Ok(()) without performing
any real check. In Soroban SDK v21, the Address type is constructed and
validated by the host environment before contract code executes — an
invalid or zero address cannot be passed in.

Changes:
- Remove validate_address function from validation.rs
- Add a comment block explaining why no address validation is needed
- Remove all call sites in validate_initialize_request,
  validate_create_remittance_request, validate_confirm_payout_request,
  validate_cancel_remittance_request, validate_withdraw_fees_request,
  validate_admin_operation, and the batch settlement loop in lib.rs
- Prefix now-unused parameters with _ to suppress compiler warnings
- Remove the now-redundant test_validate_valid_address test

Closes #<issue>
feat(contract): expose admin/token queries, harden init-required getters, and add expired remittance batch refunds
…nce-created-webhook

Feat/remittance created webhook
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.