Skip to content

fix(simulate): Use core ledger close time#742

Open
jia57b wants to merge 1 commit into
stellar:mainfrom
jia57b:fix-simulate-core-ledger-close-time
Open

fix(simulate): Use core ledger close time#742
jia57b wants to merge 1 commit into
stellar:mainfrom
jia57b:fix-simulate-core-ledger-close-time

Conversation

@jia57b

@jia57b jia57b commented May 18, 2026

Copy link
Copy Markdown

Summary

Use the latest ingested core ledger close time for preflight simulation instead of the local wall-clock time.

This change aligns simulateTransaction with the actual ledger timestamp observed by stellar-core, which is especially important for tests and workflows that manipulate ledger time
(for example via forceclosetime). Previously, simulation used time.Now(), which could drift from the real chain time and produce inconsistent results between simulation and on-chain execution.

What changed

  • Added GetLatestLedgerTime(ctx) to the LedgerReader interface.
  • Implemented latest-ledger-close-time retrieval using the existing DB cache / ledger range data.
  • Threaded the latest ledger close time through the preflight worker pool and preflight parameter structs.
  • Updated preflight ledger info construction to use the ingested core ledger close time instead of the host system clock.
  • Updated mocks and unit tests to support the new ledger-time lookup path.

Why

simulateTransaction should execute against the same time context as the latest ledger known to RPC, not the machine's current time. Using wall-clock time can make simulation:

  • disagree with core when ledger time has been forced or drifted
  • produce timestamps that do not match submitted transaction behavior
  • cause flaky integration tests around ledger-time-sensitive contracts

With this change, simulation uses the same ledger-close-time source that RPC ingests from core, making simulation results deterministic and consistent with chain state.

Testing

  • Updated unit tests
  • Run unit tests: go test ./cmd/stellar-rpc/internal/methods/... ./cmd/stellar-rpc/internal/preflight/... ./cmd/stellar-rpc/internal/db/...

(cherry picked from commit c05d154b82713fd098f58962c391d33c4ca21553)
Copilot AI review requested due to automatic review settings May 18, 2026 07:17

Copilot AI left a comment

Copy link
Copy Markdown

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 updates simulation preflight to use the latest ingested ledger close time instead of the host wall clock, aligning simulated execution with the ledger context known to RPC.

Changes:

  • Adds ledger close time to preflight parameter plumbing.
  • Extracts bucket list size, protocol version, and close time from latest ledger metadata.
  • Adds unit tests covering ledger-time propagation and metadata error paths.

Reviewed changes

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

Show a summary per file
File Description
cmd/stellar-rpc/internal/preflight/preflight.go Uses provided ledger time when constructing C ledger info.
cmd/stellar-rpc/internal/preflight/preflight_test.go Adds coverage for ledger info timestamp behavior.
cmd/stellar-rpc/internal/preflight/pool.go Threads ledger time through worker pool parameters.
cmd/stellar-rpc/internal/methods/simulate_transaction.go Reads latest ledger metadata and passes close time to preflight.
cmd/stellar-rpc/internal/methods/simulate_transaction_test.go Adds handler tests for ledger-time propagation and error handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@leighmcculloch leighmcculloch requested a review from a team May 18, 2026 14:44

@Shaptic Shaptic 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.

This is great, thank you! Would you mind just adding a line under a new ### Fixed header in the Unreleased section of the changelog as a quick release note?

@leighmcculloch

Copy link
Copy Markdown
Member

In addition to the ask about by @Shaptic the lints appear to be failing and would need resolving before merging.

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.

4 participants