Skip to content

Conversation

@anupsdf
Copy link
Contributor

@anupsdf anupsdf commented Dec 4, 2025

what

Added documention for core invariants and fixed example config

why

Improve documentation

Copilot AI review requested due to automatic review settings December 4, 2025 00:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive documentation for core invariants in stellar-core and updates the example configuration file to reflect currently available invariants.

  • Adds a new docs/invariants.md file documenting all invariant types, their purposes, implementation details, and configuration options
  • Updates docs/stellar-core_example.cfg to remove outdated CacheIsConsistentWithDatabase invariant and reorder existing invariants with improved descriptions
  • Documents three invariants with updated descriptions: ConservationOfLumens, ConstantProductInvariant, and adds new SponsorshipCountIsValid

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
docs/stellar-core_example.cfg Removes obsolete CacheIsConsistentWithDatabase entry, reorders invariants alphabetically, and adds documentation for ConstantProductInvariant and SponsorshipCountIsValid with improved descriptions
docs/invariants.md Adds comprehensive documentation covering all 10 invariants with architectural overview, check methods, configuration instructions, and best practices for adding new invariants

Copy link
Contributor

@SirTyson SirTyson left a comment

Choose a reason for hiding this comment

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

Overall LGTM. I would add a big warning at the top of this though that invariants are very expensive, may reduce performance, and will crash your node if a fault is detected. Basically just a section to dissuade tier 1 from actually turning these on for production validators.

|--------|-------------|
| `checkOnOperationApply` | After each operation is applied |
| `checkOnBucketApply` | After bucket apply during catchup |
| `checkAfterAssumeState` | After assuming state from history |
Copy link
Contributor

Choose a reason for hiding this comment

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

assume state runs on every restarts, not just when catching up from history

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

- Only applies to Soroban entry types (CONTRACT_CODE, CONTRACT_DATA)
- Verifies that no entry exists in both the live state and the archived (Hot Archive) state simultaneously
- Checks eviction invariants: entries evicted from live state should not already exist in the hot archive
- Checks restoration invariants: entries restored from hot archive should match expected states
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: restoration invariant checks that restored entries were not already in live state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


**File:** `BucketListIsConsistentWithDatabase.cpp`

**Purpose:** Validates that the BucketList and Database are in a consistent state after bucket operations.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a note that this only applies to OFFER types, as all non-offer LIVEENTRY are not reflected in the database.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

**Details:**
- Iterates over bucket entries and compares them to the database
- Fails if:
- A LIVEENTRY in the bucket doesn't match the database (not present or different)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the references to LIVEENTRY and DEADENTRY. It doesn't make sense to talk about individual Bucket types here, since an entry can be shadowed, so the existence or nonexistence of a LIVEENTRY/DEADENTRY isn't particularly relevant. I would just say "check that there is a one-to-one mapping of OFFER entry types between SQL DB and BucketList state".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

- For AMM (Automated Market Maker) pools using constant product formula
- Validates: `currentReserveA * currentReserveB >= previousReserveA * previousReserveB`
- Excludes `LIQUIDITY_POOL_WITHDRAW`, `SET_TRUST_LINE_FLAGS`, and `ALLOW_TRUST` operations (which intentionally may decrease the product)
- Prevents manipulation that would drain liquidity unfairly
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of "prevent" I would say "detects"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.


## Configuration

Invariants can be enabled through stellar-core configuration. The `InvariantManager` supports:
Copy link
Contributor

Choose a reason for hiding this comment

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

This section talks about what the interface of the InvariantManager class does and doesn't really have anything to do with configuration. I would just remove it and have the section below become "configuration"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Edited.


### State Snapshot Invariants

Some expensive invariants (like `ArchivedStateConsistency.checkSnapshot`) run periodically on a background thread against ledger state snapshots, as they require scanning the entire BucketList and can't run in a blocking fashion during normal operation.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should note that enabling snapshot invariants is very expensive and requires lot's of memory. For this reason, it is not allowed on validator nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note.


## Best Practices for Adding New Invariants

1. **Choose the right check method**: Select based on when the validation should occur
Copy link
Contributor

Choose a reason for hiding this comment

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

Item 1 and 3 aren't particularly helpful and are to niche.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleaned up.

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.

2 participants