Skip to content

Contract fixes: zero merkle root guard, in-place pending removal, snapshotted confirmation count in failure event - #185

Merged
terngunan merged 4 commits into
terngunan:mainfrom
josunday002:drips/97-98-99
Oct 21, 2026
Merged

terngunan merged 4 commits into
terngunan:mainfrom
josunday002:drips/97-98-99

Conversation

@josunday002

Copy link
Copy Markdown
Contributor

Summary

Contract fixes: zero merkle root guard, in-place pending removal, snapshotted confirmation count in failure event

What was solved

#97 — reconciliation: Missing non-zero validation for submitted Merkle roots allows recording empty or null roots

Add a non-zero validation guard to submit_merkle_root in the reconciliation contract so all-zero Merkle roots are rejected, and add a unit test asserting the panic on a zero root while confirming valid non-zero roots still succeed.

Addressed:

  • Changed: settliq_contracts/contracts/reconciliation/src/lib.rs, settliq_contracts/contracts/reconciliation/src/test.rs
  • In settliq_contracts/contracts/reconciliation/src/lib.rs, add a check in submit_merkle_root that panics with a clear message (e.g. "merkle root must not be zero") when merkle_root == BytesN::from_array(&env, &[0u8; 32]).
  • Ensure valid non-zero Merkle roots continue to be accepted and recorded as before.
  • Add a unit test in settliq_contracts/contracts/reconciliation/src/test.rs that submits an all-zero 32-byte array and asserts the call panics/rejects.

#98 — admin_timelock: Quadratic memory allocation in remove_from_pending when modifying pending changes list

Refactor remove_from_pending in the admin_timelock contract to mutate the existing PendingList vector in place using first_index_of + remove (or swap_remove) instead of allocating a second vector and copying every non-matching ID. Add unit tests in the contract's test module that schedule and apply/cancel multiple changes and assert list_pending reflects the correct state.

Addressed:

  • Changed: settliq_contracts/contracts/admin_timelock/src/lib.rs
  • Modify remove_from_pending in settliq_contracts/contracts/admin_timelock/src/lib.rs to locate the target index via first_index_of and remove in place (e.g. pending.remove(pos) or swap_remove) rather than building a new Vec and copying all non-matching IDs.
  • Ensure the pending list correctly excludes the applied or cancelled change after apply_change and cancel_change.
  • Add unit tests in settliq_contracts/contracts/admin_timelock/src/test.rs that schedule multiple changes, apply/cancel some, and verify list_pending state.

#99 — stellar_confirmations: fail_confirmation publishes global confirmation count instead of the payment's snapshotted threshold

Fix fail_confirmation in the stellar_confirmations contract so the ConfirmationFailedEvent publishes the payment's snapshotted required confirmation count (DataKey::PaymentRequiredConfs(payment_id)) instead of the current global ConfirmationCount, falling back to the global value only when no snapshot exists. Add a unit test that updates the global threshold after one confirmation, fails the payment, and asserts the event payload reflects the snapshotted threshold.

Addressed:

  • Changed: settliq_contracts/contracts/stellar_confirmations/src/lib.rs, settliq_contracts/contracts/stellar_confirmations/src/test.rs
  • In fail_confirmation, read the required count from DataKey::PaymentRequiredConfs(payment_id) (persistent storage) and fall back to DataKey::ConfirmationCount (instance storage) only when no snapshot is present.
  • ConfirmationFailedEvent.required must report the payment's snapshotted required confirmations.
  • Fallback to global ConfirmationCount when no confirmations were recorded (no snapshot).

Changes

  • settliq_contracts/contracts/reconciliation/src/test.rs (modify)
  • settliq_contracts/contracts/stellar_confirmations/src/lib.rs (modify)
  • settliq_contracts/contracts/reconciliation/src/lib.rs (modify)
  • settliq_contracts/contracts/admin_timelock/src/lib.rs (modify)
  • settliq_contracts/contracts/stellar_confirmations/src/test.rs (modify)

Approach

  1. reconciliation: Missing non-zero validation for submitted Merkle roots allows recording empty or null roots #97 — reconciliation: Missing non-zero validation for submitted Merkle roots allows recording empty or null roots (Changed: settliq_contracts/contracts/reconciliation/src/lib.rs, settliq_contracts/contracts/reconciliation/src/test.rs)
  2. admin_timelock: Quadratic memory allocation in remove_from_pending when modifying pending changes list #98 — admin_timelock: Quadratic memory allocation in remove_from_pending when modifying pending changes list (Changed: settliq_contracts/contracts/admin_timelock/src/lib.rs)
  3. stellar_confirmations: fail_confirmation publishes global confirmation count instead of the payment's snapshotted threshold #99 — stellar_confirmations: fail_confirmation publishes global confirmation count instead of the payment's snapshotted threshold (Changed: settliq_contracts/contracts/stellar_confirmations/src/lib.rs, settliq_contracts/contracts/stellar_confirmations/src/test.rs)

Issues

Closes #97
Closes #98
Closes #99

@drips-wave

drips-wave Bot commented Oct 21, 2026

Copy link
Copy Markdown

@josunday002 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! 🚀

Learn more about application limits

@terngunan
terngunan merged commit 69ec09a into terngunan:main Oct 21, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants