Skip to content

fix(contracts): index reissued certificates and mark originals as sup… - #901

Merged
feyishola merged 4 commits into
Servora:mainfrom
OmniZlatoon:fix/reissue-certificate-indexing-and-revocation
Sep 1, 2026
Merged

fix(contracts): index reissued certificates and mark originals as sup…#901
feyishola merged 4 commits into
Servora:mainfrom
OmniZlatoon:fix/reissue-certificate-indexing-and-revocation

Conversation

@OmniZlatoon

@OmniZlatoon OmniZlatoon commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes the reissue flow in the Stellar certificate contract by avoiding the ownership move error introduced when creating the replacement certificate and by ensuring the emitted CertificateReissuedEvent includes the required old_id field. The change preserves the original certificate state while correctly creating the new reissued certificate and linking it back to its parent certificate.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Files Modified

  • stellar-contracts/src/lib.rs
  • stellar-contracts/src/types.rs

Testing

  • Tested locally
  • Added unit tests
  • Tested on Stellar Testnet (for wallet/contract changes)

Code Quality checks

  • cargo build
  • cargo test
  • Warnings reviewed; no blocking compile/test failures

Behavioural Changes

  • Prevents the move error caused by new_owner.unwrap_or(original_cert.owner) by cloning the original owner when no replacement owner is supplied.
  • Ensures the reissue event includes old_id, matching the event type definition and making the parent-child relationship explicit.
  • Preserves the expected reissued certificate lifecycle semantics while keeping the contract buildable and test-passing.

Closes #802

…erseded

- Add new certificate to IssuerCertIds and OwnerCertIds indexes
- Mark original certificate status as Revoked with reason 'Superseded'
- Emit CertificateRevokedEvent for original certificate
- Update test_reissue_certificate to verify all three requirements

Fixes: reissue_certificate does not index the new certificate and leaves the old one active

The fix ensures:
1. Reissued certificates are discoverable via get_certificates_by_issuer/owner
2. Original certificates are marked as revoked (not simultaneously active)
3. Proper audit trail with revocation events for both old and new certs
@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@OmniZlatoon 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

@OmniZlatoon

Copy link
Copy Markdown
Contributor Author

Good day sir @feyishola , please kindly review the PR

@feyishola

Copy link
Copy Markdown
Contributor

@OmniZlatoon

(index reissued certs)

The approach is right, but it doesn't compile: new_cert is built with owner: new_owner.unwrap_or(original_cert.owner), which partially moves original_cert, so the later let mut updated_original = original_cert; is a use-of-moved-value (E0382). Fix: clone the field — new_owner.unwrap_or(original_cert.owner.clone()) — or restructure so original_cert isn't moved first. Please run cargo build && cargo test before pushing (it can't currently pass).

@OmniZlatoon

Copy link
Copy Markdown
Contributor Author

Good day sir @feyishola , please kindly review the PR

3 similar comments
@OmniZlatoon

Copy link
Copy Markdown
Contributor Author

Good day sir @feyishola , please kindly review the PR

@OmniZlatoon

Copy link
Copy Markdown
Contributor Author

Good day sir @feyishola , please kindly review the PR

@OmniZlatoon

Copy link
Copy Markdown
Contributor Author

Good day sir @feyishola , please kindly review the PR

@feyishola feyishola left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — thanks for contributing! 🎉

@feyishola
feyishola merged commit a63e9d2 into Servora:main Sep 1, 2026
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.

reissue_certificate does not index the new certificate and leaves the old one active

2 participants