feat: Validate auth entry signature expiration ledger#84
Conversation
WalkthroughThis PR adds signature expiration validation for Soroban authorization entries by introducing a configurable minimum ledger buffer parameter. Configuration is extended to load and pass Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as Handler (func-auth)
participant Config as Config<br/>(loadConfig)
participant Simulation as buildWithChannel
participant Validator as validateAuthExpiry
participant Ledger as Soroban Network
Handler->>Config: loadConfig()
Config-->>Handler: ChannelAccountsConfig<br/>(with minSignatureExpirationLedgerBuffer)
Handler->>Simulation: buildWithChannel(<br/>..., minBuffer)
Simulation->>Ledger: Get latestLedger<br/>(from simResult)
Ledger-->>Simulation: latestLedger number
Simulation->>Validator: validateAuthExpiry(<br/>authEntries,<br/>latestLedger,<br/>minBuffer)
alt Expiry Too Close
Validator-->>Simulation: Throw AUTH_EXPIRY_TOO_SHORT<br/>(with margin details)
else Expiry Valid
Validator-->>Simulation: OK, continue
end
Simulation-->>Handler: Transaction or Error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/config.test.ts (1)
146-165: Add a test for value1to enforce the minimum buffer.If the minimum buffer is truly “>= 2,” adding a case for
MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER=1will prevent regressions once the parser is tightened.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/config.test.ts` around lines 146 - 165, Add a test case in test/config.test.ts to cover MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER='1' so the minimum buffer requirement is enforced: when process.env.MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER = '1' assert that loadConfig().minSignatureExpirationLedgerBuffer === 2 (same fallback as other invalid/too-small values), locating this near the existing "min auth expiry ledger buffer" test that references loadConfig and minSignatureExpirationLedgerBuffer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugin/config.ts`:
- Around line 111-116: The parseMinAuthExpiryLedgerBuffer function currently
accepts any positive number allowing 1, but must enforce a minimum of
CONFIG.DEFAULT_MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER (>=2); update
parseMinAuthExpiryLedgerBuffer to parse the env var into n, clamp it so the
returned value is Math.floor(n) only when Number.isFinite(n) and Math.floor(n)
>= CONFIG.DEFAULT_MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER, otherwise return
CONFIG.DEFAULT_MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER; reference the function
name parseMinAuthExpiryLedgerBuffer and the constant
CONFIG.DEFAULT_MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER when making the change.
---
Nitpick comments:
In `@test/config.test.ts`:
- Around line 146-165: Add a test case in test/config.test.ts to cover
MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER='1' so the minimum buffer requirement is
enforced: when process.env.MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER = '1' assert
that loadConfig().minSignatureExpirationLedgerBuffer === 2 (same fallback as
other invalid/too-small values), locating this near the existing "min auth
expiry ledger buffer" test that references loadConfig and
minSignatureExpirationLedgerBuffer.
There was a problem hiding this comment.
Pull request overview
Adds a “fail fast” validation layer to reject Soroban auth entries whose signatureExpirationLedger is too close to the simulation’s latestLedger, with configuration and documentation support.
Changes:
- Add auth expiry margin validation in
buildWithChannel, throwingAUTH_EXPIRY_TOO_SHORTwhen the margin is below a configurable buffer (default 2). - Expose
MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFERvia config/env and plumb it through the request handler. - Add tests + README updates to document the new config and error code.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/plugin/simulation.ts |
Validates auth expiry margin before assembling transactions; introduces AUTH_EXPIRY_TOO_SHORT. |
src/plugin/handler.ts |
Threads config through pipeline and passes minSignatureExpirationLedgerBuffer into buildWithChannel. |
src/plugin/config.ts |
Adds parsing + config field for MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER. |
src/plugin/constants.ts |
Introduces defaults/constants for the new buffer. |
test/read-only.test.ts |
Adds coverage for rejection/acceptance based on expiry margin + buffer. |
test/config.test.ts |
Adds coverage for parsing MIN_SIGNATURE_EXPIRATION_LEDGER_BUFFER. |
README.md |
Documents env var and error code categorization additions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR adds validation for the auth entry signature expiration ledger value.
Requests with auth entries whose expiration is too close to the current ledger will be rejected. The minimum signature ledger buffer is 2, ensuring there is enough time to process the transaction.
Summary by CodeRabbit
New Features
Documentation