feat: Bruno API collection (#281) and Soroban TTL extension (#282)#295
Conversation
📝 WalkthroughWalkthroughExtends contract storage TTLs for admin, expert, and session keys, adds configurable minimum and maximum TTL ledger settings with public admin entrypoints, and adds a Bruno API collection with environment files and request specs for the contract endpoints. ChangesSoroban TTL extension
Bruno API collection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (48)
contracts/src/storage.rs-28-56 (1)
28-56: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTTL config setters store values without ordering validation.
set_min_ttl_ledgers/set_max_ttl_ledgerspersist raw ledger counts with no guarantee thatmin <= max. Sinceextend_persistent_ttl/extend_instance_ttlbelow feed these asextend_ttl(key, min, max)(i.e.threshold,extend_to), a configuration wheremin > maxmakes every subsequent extension call abort. The root-cause fix belongs in the admin entrypoints — see the comment oncontracts/src/lib.rsLines 1755-1793.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/src/storage.rs` around lines 28 - 56, The TTL setters currently persist raw values without enforcing a valid range, so fix the admin flow that calls set_min_ttl_ledgers and set_max_ttl_ledgers to reject or normalize configurations where min_ttl_ledgers would exceed max_ttl_ledgers. Add the ordering check in the admin entrypoints that configure TTLs, using the existing min_ttl_ledgers and max_ttl_ledgers helpers as the source of truth, so extend_persistent_ttl and extend_instance_ttl always pass a valid threshold/extend_to pair into extend_ttl.docs/api/04-Session-Lifecycle/top_up_session.bru-32-34 (1)
32-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd a real
sendTransactionshape check here.Lines 32-34 only confirm that
jsonrpcexists. They do not prove the transaction was accepted or that the response is well-formed, becauseresult.hash,result.status, and top-levelerrorare never checked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/04-Session-Lifecycle/top_up_session.bru` around lines 32 - 34, The “Valid response” test in top_up_session.bru is too shallow because it only checks jsonrpc and does not verify that sendTransaction succeeded or returned a valid payload. Update the test to assert the full sendTransaction response shape, using the existing test block that inspects response, by checking result.hash, result.status, and ensuring top-level error is absent so the response is actually well-formed and accepted.docs/api/03-Expert-Management/set_availability.bru-32-34 (1)
32-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTighten the response-shape test beyond
resultexistence.Lines 32-34 only check that a
resultproperty exists, which still passes forresult: nullor other incomplete payloads. This request should validate the standardsendTransactionshape (jsonrpc,result.hash,result.status) and fail on top-levelerror.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/03-Expert-Management/set_availability.bru` around lines 32 - 34, The response-shape assertion in the `Transaction submitted` test is too weak because it only checks for `response.result` existence. Update this test to validate the full `sendTransaction`-style payload by asserting `jsonrpc` is present, `result.hash` and `result.status` exist, and that no top-level `error` field is returned. Use the existing `response` object in the `test("Transaction submitted", ...)` block to make the checks stricter and reject null or incomplete results.docs/api/03-Expert-Management/update_agency_split.bru-32-34 (1)
32-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the transaction payload, not just the JSON-RPC version.
Lines 32-34 only assert
"2.0", so a response can still pass without a transaction hash/status or with an unchecked top-levelerror. This falls short of the response-shape coverage the collection is supposed to provide.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/03-Expert-Management/update_agency_split.bru` around lines 32 - 34, The current check in the response test only validates response.jsonrpc, which is too weak for the transaction flow. Update the test around test("Valid JSON-RPC response") to assert the full transaction payload shape, including the expected transaction hash/status fields and that the top-level error is absent or handled explicitly. Use the existing response object in update_agency_split.bru to strengthen coverage so a malformed transaction response cannot still pass.docs/api/03-Expert-Management/register_expert.bru-32-38 (1)
32-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire
result.hashinstead of checking it conditionally.Lines 32-35 skip the shape assertion whenever
response.resultis missing, so a malformed success payload can still pass as long as there is no top-levelerror. For this collection,sendTransactionrequests should fail unless the response contains the expectedresultshape.Suggested tightening
test("Transaction hash present", () => { - if (response.result) { - expect(response.result).to.have.property("hash"); - } + expect(response).to.have.property("result"); + expect(response.result).to.have.property("hash"); + expect(response.result.hash).to.match(/^[a-f0-9]{64}$/); + expect(["PENDING", "SUCCESS"]).to.include(response.result.status); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/03-Expert-Management/register_expert.bru` around lines 32 - 38, The response validation in the register_expert request is too loose because the `Transaction hash present` test only runs when `response.result` exists. Tighten the assertion in this test block by requiring `response.result.hash` directly and letting the test fail when `result` is missing or malformed; use the existing `response` checks in this `test(...)` section to enforce the expected `sendTransaction` response shape instead of conditionally skipping it.docs/api/03-Expert-Management/heartbeat.bru-27-35 (1)
27-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStrengthen the
sendTransactionresponse assertions.Lines 27-35 only prove that the server returned
200and somejsonrpcfield. They never verifyresult.hash/result.statusor the absence of top-levelerror, so this request does not meet the collection’s required response-shape coverage.Suggested tightening
tests { const response = res.getBody(); test("Response status 200", () => { expect(res.status).to.equal(200); }); - test("Heartbeat submitted", () => { - expect(response).to.have.property("jsonrpc"); - }); + test("Valid JSON-RPC response", () => { + expect(response.jsonrpc).to.equal("2.0"); + expect(response).to.have.property("result"); + expect(response.result).to.have.property("hash"); + expect(response.result.hash).to.match(/^[a-f0-9]{64}$/); + expect(["PENDING", "SUCCESS"]).to.include(response.result.status); + }); + test("No RPC error", () => { + expect(response).to.not.have.property("error"); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/03-Expert-Management/heartbeat.bru` around lines 27 - 35, The heartbeat request tests in the tests block only assert HTTP 200 and the presence of jsonrpc, so tighten the response-shape coverage by updating the assertions around res.getBody() in heartbeat.bru to verify result.hash and result.status are present and to confirm there is no top-level error field. Keep the existing sendTransaction/heartbeat response checks aligned with the collection’s expected schema rather than only checking the transport status.docs/api/03-Expert-Management/get_expert_profile.bru-32-42 (1)
32-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the response-shape checks mandatory.
Lines 32-42 only assert inside
ifguards, so a200response with a missingresultor a top-level JSON-RPCerrorstill passes. This misses the PR’s required response-shape validation, anderrorshould be checked onresponse, not underresponse.result.results[0].Suggested tightening
tests { const response = res.getBody(); test("Response status 200", () => { expect(res.status).to.equal(200); }); - test("Simulation succeeded", () => { - if (response.result && response.result.results) { - const result = response.result.results[0]; - expect(result).to.have.property("footprint"); - } - }); - test("No contract error", () => { - if (response.result && response.result.results) { - const error = response.result.results[0]?.error; - expect(error).to.be.undefined; - } - }); + test("Valid JSON-RPC response shape", () => { + expect(response.jsonrpc).to.equal("2.0"); + expect(response).to.have.property("result"); + expect(response.result).to.have.property("results"); + expect(response.result.results).to.be.an("array").that.is.not.empty; + expect(response.result.results[0]).to.have.property("footprint"); + }); + test("No RPC error", () => { + expect(response).to.not.have.property("error"); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/03-Expert-Management/get_expert_profile.bru` around lines 32 - 42, The response-shape validation in the expert profile simulation tests is only happening inside `if (response.result && response.result.results)` guards, so missing `result` or a top-level JSON-RPC `error` can still pass. Update the checks in `get_expert_profile.bru` so `test("Simulation succeeded")` and `test("No contract error")` assert the required shape unconditionally, and verify `response.error` directly instead of reading `response.result.results[0]?.error`; use the existing `response` and `response.result.results` references to tighten the assertions.docs/api/04-Session-Lifecycle/start_session.bru-32-39 (1)
32-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStart-session acceptance is never actually asserted.
The only transaction-level check sits behind a conditional, and the last test only rules out top-level JSON-RPC errors. A payload with
result.statusindicating failure would still pass here. Requireresponse.resultand validate its status/hash directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/04-Session-Lifecycle/start_session.bru` around lines 32 - 39, The start_session acceptance is not actually being asserted because the current checks in start_session.bru are conditional and only exclude top-level errors. Update the tests around the response assertion so they require response.result to exist and directly verify response.result.status is SUCCESS along with the presence of response.result.hash, rather than skipping the check when result is missing or failed.docs/api/04-Session-Lifecycle/settle_session.bru-32-40 (1)
32-40: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe settlement “success” check is currently vacuous.
The
hashassertion only runs when theifcondition is already true, so a missing or unsuccessfulresultstill leaves this file green because the final test only checksjsonrpcandid. Assert the transaction result first, then validate the expected status fields unconditionally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/04-Session-Lifecycle/settle_session.bru` around lines 32 - 40, The settlement success test is currently too weak because the hash assertion is guarded by a conditional, so failures can still pass unnoticed. Update the settlement checks in settle_session.bru so the transaction result is asserted directly and unconditionally before any field validation, using the existing response.result and status/hash expectations in the “Settlement result present” and “Result shape is valid” tests. Ensure the test fails when result is missing, unsuccessful, or lacks the expected hash instead of skipping the assertion.docs/api/04-Session-Lifecycle/pause_resume_session.bru-27-34 (1)
27-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThese tests still pass on RPC failures.
A JSON-RPC error response can be HTTP 200 and still include
jsonrpc, so both checks can stay green while pause/resume never succeeds. Assertresponse.resultand reject top-levelerrorso this request actually validates the operation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/04-Session-Lifecycle/pause_resume_session.bru` around lines 27 - 34, The current pause/resume session tests only verify HTTP 200 and the presence of jsonrpc, so they can still pass on JSON-RPC failures. Update the tests in the pause/resume session request to assert a successful response by checking response.result and explicitly failing when a top-level error is present, using the existing res and response assertions in the tests block so the operation is actually validated.docs/api/04-Session-Lifecycle/calculate_claimable.bru-18-23 (1)
18-23: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable sample.
transactionis still an ellipsis placeholder, so this request cannot be executed in Bruno as checked in. That falls short of the “ready-to-use against Testnet” objective for the collection; use an env-backed base64 envelope or a checked-in sample XDR that actually decodes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/04-Session-Lifecycle/calculate_claimable.bru` around lines 18 - 23, The calculate_claimable Bruno request still uses a placeholder value for the transaction XDR, so it cannot be executed as checked in. Update the request’s params in calculate_claimable.bru by replacing the ellipsis-based placeholder with a runnable base64-encoded sample or an env-backed value that decodes correctly, using the existing transaction field and resourceConfig block so the request is ready to run against Testnet.docs/api/testnet.env-3-3 (1)
3-3: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIncorrect testnet network passphrase — should be "September 2015", not 2025.
The Soroban testnet passphrase is
Test SDF Network ; September 2015. The2025value will break transaction signing against testnet.Proposed fix
-networkPassphrase=Test SDF Network ; September 2025 +networkPassphrase=Test SDF Network ; September 2015🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/testnet.env` at line 3, The networkPassphrase value is incorrect for the testnet config and should use the Soroban testnet passphrase expected by transaction signing. Update the networkPassphrase entry in testnet.env to match the correct Test SDF Network value with “September 2015” instead of “September 2025”.docs/api/mainnet.env-3-3 (1)
3-3: 🎯 Functional Correctness | 🟠 MajorFix the mainnet network passphrase —
docs/api/mainnet.env:3should usePublic Global Stellar Network ; September 2015, notSeptember 2025. The wrong passphrase will produce signatures for a different network, and mainnet transactions will be rejected.Proposed fix
-networkPassphrase=Public Global Stellar Network ; September 2025 +networkPassphrase=Public Global Stellar Network ; September 2015🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/mainnet.env` at line 3, The mainnet environment value is using the wrong Stellar network passphrase, so update the `networkPassphrase` entry in `mainnet.env` to the correct mainnet string. Use the existing `networkPassphrase` setting as the locator and replace the `September 2025` value with the proper `Public Global Stellar Network ; September 2015` passphrase so signatures target the correct network.docs/api/02-Admin/set_burn_bps.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 still contains example text instead of a valid transaction XDR, so this request is not ready to run in Bruno.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_burn_bps.bru` at line 19, The transaction field in set_burn_bps.bru still uses placeholder text instead of a runnable XDR fixture. Replace the example value in the request body with a valid base64-encoded transaction XDR that Bruno can execute, using the transaction entry in this API fixture as the target to update.docs/api/02-Admin/set_insurance_vault.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 is still a dummy transaction string, so this request is not usable for real insurance-vault testing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_insurance_vault.bru` at line 19, The set_insurance_vault fixture still uses a placeholder transaction XDR, so replace the dummy value in the request body with a real runnable transaction payload that can be used for insurance-vault testing. Update the transaction field in set_insurance_vault.bru to reference a valid fixture/XDR generated for this API flow, and ensure the request remains directly runnable without manual editing.docs/api/02-Admin/set_insurance_vault.bru-27-34 (1)
27-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd a concrete
resultassertion.A 200 response without a top-level
erroris not enough to prove the request succeeded. This should also validate the expectedsendTransactionresult shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_insurance_vault.bru` around lines 27 - 34, The current test in set_insurance_vault.bru only checks for a 200 status and absence of a top-level error, so it does not verify success fully. Update the existing tests block to assert the concrete expected result payload from the `sendTransaction` flow, using the response body (`res.getBody()`) and the relevant `result` field shape/value in addition to the current status check.docs/api/02-Admin/initialize.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 is still a placeholder transaction, so the initialize request is not actually executable from Bruno.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/initialize.bru` at line 19, The initialize Bruno fixture still uses a placeholder transaction value, so the request cannot run as-is. Replace the current transaction field in initialize.bru with a real runnable XDR fixture that Bruno can execute, and make sure the initialize request content references the concrete fixture data rather than a base64 placeholder.docs/api/02-Admin/set_fee_tiers.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 is placeholder text rather than a valid signed XDR, so this fee-tier request is not actually runnable from Bruno.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_fee_tiers.bru` at line 19, The set_fee_tiers Bruno fixture still uses a placeholder value for the transaction field instead of a runnable signed XDR. Update the request payload in set_fee_tiers.bru so the transaction entry contains a real, valid base64-encoded signed XDR that can be executed from Bruno, and keep the fixture aligned with the fee-tier request shape used by this API example.docs/api/02-Admin/grant_revoke_role.bru-32-37 (1)
32-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis success check is effectively a no-op.
The assertions are gated behind
ifbranches, andevents.length >= 0is always true. A 200 response with no simulation results will still pass. Assert thatresponse.result.results[0]exists before checking a concrete field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/grant_revoke_role.bru` around lines 32 - 37, The success test in the role management simulation is too weak because the assertions are hidden behind nested conditionals and `events.length >= 0` always passes. Update the `test("Role management simulation succeeded", ...)` block to explicitly assert that `response.result.results[0]` exists before inspecting `events`, and then check a meaningful condition on that field rather than a no-op length check. Use the `response.result.results` and `events` references in this test to tighten the validation.docs/api/02-Admin/set_fee.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 is still placeholder data, so this request cannot be executed against the RPC endpoint as documented.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_fee.bru` at line 19, The set_fee.bru request still contains placeholder XDR data, so replace the transaction field with a real runnable fixture that can be submitted to the RPC endpoint. Update the example payload in set_fee.bru to use a valid base64-encoded transaction XDR for the set_fee flow, keeping the request structure intact and ensuring the fixture is executable as documented.docs/api/02-Admin/grant_revoke_role.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 still uses a fake
transactionvalue, so this request cannot be executed in Bruno as-is. That misses the ready-to-use collection objective.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/grant_revoke_role.bru` at line 19, The grant_revoke_role Bruno request still uses a placeholder transaction XDR, so the collection is not runnable as-is. Replace the fake `transaction` value in grant_revoke_role.bru with a real fixture that can be executed in Bruno, and make sure the updated request uses a valid base64-encoded XDR payload consistent with the surrounding admin API examples.docs/api/02-Admin/pause_unpause.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 is a placeholder transaction, so this request cannot be used for manual pause/unpause testing in its current form.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/pause_unpause.bru` at line 19, The pause/unpause request in the bru fixture still uses a placeholder transaction value, so it cannot be executed for manual testing. Replace the dummy XDR in the pause_unpause.bru fixture with a real runnable transaction payload that matches the Admin pause/unpause flow, and make sure the request remains valid for the relevant endpoint and the existing transaction field structure.docs/api/02-Admin/set_burn_bps.bru-32-34 (1)
32-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
jsonrpcalone is not a success shape.A JSON-RPC error payload still satisfies this assertion. To meet the response-shape requirement, assert a
sendTransactionresult field such asresult.hashorresult.status.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_burn_bps.bru` around lines 32 - 34, The current response-shape check in the set_burn_bps.bru test only asserts jsonrpc, which also passes for error payloads. Update the test to validate the sendTransaction success shape using a real result field such as result.hash or result.status, and keep the assertion tied to the response object in the “Valid response shape” test.docs/api/02-Admin/set_fee_tiers.bru-32-35 (1)
32-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe response-shape assertion is too weak.
jsonrpcandidare present on error responses too, so these checks do not verify thatsendTransactionsucceeded. Assert a concreteresultshape here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_fee_tiers.bru` around lines 32 - 35, The response-shape test in the set_fee_tiers response checks is too weak because jsonrpc and id exist on error payloads too. Update the assertion in the Valid response shape test for sendTransaction to verify a concrete successful result structure using response.result, and reference the existing response variable so the test clearly distinguishes success from error responses.docs/api/02-Admin/set_fee.bru-32-35 (1)
32-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis test passes even when no transaction result is returned.
Because the assertion is wrapped in
if (response.result), a 200 response with an empty or malformed body still passes. Requireresponse.result.hashinstead of checking it conditionally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_fee.bru` around lines 32 - 35, The transaction acceptance test in the set_fee Bru file is too permissive because the hash assertion is only executed inside a conditional on response.result. Update the test("Transaction accepted") block to assert response.result.hash directly so the check fails when no transaction result is returned or the body is malformed, and keep the fix localized to the response.result validation in that test.docs/api/02-Admin/set_admin.bru-27-35 (1)
27-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd a real success-shape assertion.
These tests only prove that the transport returned 200 and omitted a top-level
error. They never verify the expectedsendTransactionpayload, so the request can still pass without a usable result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_admin.bru` around lines 27 - 35, The current tests in set_admin.bru only check HTTP 200 and that a top-level error field is absent, but they never validate the actual success payload. Update the tests inside the `tests` block to assert the expected `sendTransaction` response shape from `res.getBody()`, using a concrete success property or structure from the admin flow so the request cannot pass without a usable result.docs/api/02-Admin/initialize.bru-35-45 (1)
35-45: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe response-shape checks never fail on missing
result.Both tests are conditional on
response.result, so a malformed or error payload can still pass as long as the HTTP status is 200. Makeresult.hashandresult.statusrequired for this request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/initialize.bru` around lines 35 - 45, The response-shape assertions in initialize.bru are too permissive because both checks are gated by response.result, so missing result data can still pass. Update the tests around response.result so they fail when result, hash, or status are absent, and keep the existing hash/status validation tied to the initialize request response. Use the existing response.result, "Transaction hash returned", and "Status indicates pending or success" checks as the location to make result.hash and result.status required.docs/api/02-Admin/set_admin.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder XDR with a runnable fixture.
Line 19 is not a real transaction XDR, so this admin-transfer request is not usable as delivered.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/02-Admin/set_admin.bru` at line 19, The admin-transfer request in set_admin.bru still uses a placeholder transaction XDR, so replace the current transaction value with a real runnable fixture that can be submitted as-is. Update the transaction field in the set_admin request example to use an actual valid XDR payload that matches the documented admin transfer flow, keeping the rest of the example consistent and executable.docs/api/05-Disputes/add_dispute_evidence.bru-13-24 (1)
13-24: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the stub XDR with an executable example.
This payload is still a literal placeholder, so the request cannot actually exercise
add_dispute_evidenceas a ready-to-use Bruno example. Please either parameterize/build the transaction from env vars or document a reproducible pre-request step that generates the signed XDR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/05-Disputes/add_dispute_evidence.bru` around lines 13 - 24, The example in add_dispute_evidence is still using a placeholder transaction payload, so update the Bruno request to use a real executable flow. In the body for sendTransaction, replace the stub XDR with either an env-driven transaction assembled from actual inputs or a transaction generated by a documented pre-request script, and make sure the request references the same add_dispute_evidence example so it can be run as-is. If you choose pre-request generation, add the necessary step in the Bruno request so the signed XDR is created before the sendTransaction call.docs/api/06-Staking/pending_rewards.bru-27-35 (1)
27-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssert the simulated call outcome, not just the envelope.
A failed simulation can still return HTTP 200 with
jsonrpcandid, so this test can pass whilepending_rewardsactually failed. Please assertresponse.resultand the nestedresponse.result.results[0]success/error shape here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/06-Staking/pending_rewards.bru` around lines 27 - 35, The current test only verifies the JSON-RPC envelope in pending_rewards.bru, so it can miss a failed simulation that still returns HTTP 200; update the assertions in the test block to validate the actual call outcome by checking response.result and the nested response.result.results[0] success/error shape, using the existing res.getBody() response object and the pending_rewards simulation response structure.docs/api/07-Treasury/get_treasury_balance.bru-32-39 (1)
32-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the actual simulation result entry.
Balance query succeededonly checks that some top-levelresultexists, andNo contract erroris skipped entirely whenresultsis missing. Please assertresponse.result.results[0]and its success payload explicitly so malformed simulateTransaction responses fail this request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/07-Treasury/get_treasury_balance.bru` around lines 32 - 39, The treasury balance request test is too loose because it only checks for a top-level result and skips the contract-error assertion when results is missing. Update the checks in get_treasury_balance.bru to validate response.result.results[0] directly and assert its success payload explicitly in the same test flow, so malformed simulateTransaction responses fail instead of passing silently.docs/api/07-Treasury/withdraw_insurance.bru-27-34 (1)
27-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis test passes on JSON-RPC errors.
Checking only
jsonrpcdoes not provewithdraw_insurancewas accepted; error responses include that field too. Please assert absence ofresponse.errorand presence of the expectedresponse.resultsubmission fields instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/07-Treasury/withdraw_insurance.bru` around lines 27 - 34, The response validation in the withdraw_insurance test is too weak because checking only the jsonrpc field allows JSON-RPC error payloads to pass. Update the tests block in withdraw_insurance.bru to assert that response.error is absent and that response.result contains the expected successful submission fields, using the existing response object and the withdraw_insurance request flow as the reference points.docs/api/10-Views/get_event_log.bru-32-37 (1)
32-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd assertions for the event-log payload shape.
This currently proves the JSON-RPC envelope exists, but not that the paginated event list came back in the expected structure. Please validate the returned event collection shape here instead of stopping at top-level
result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/10-Views/get_event_log.bru` around lines 32 - 37, The current checks in the event-log test only verify the JSON-RPC envelope and that a top-level result exists, but they do not validate the returned event-list payload shape. Update the assertions in the event log test block to inspect response.result and confirm the paginated collection structure and expected event fields using the existing response object in get_event_log.bru, rather than stopping at hasProperty("result").docs/api/10-Views/get_expert_tier.bru-32-34 (1)
32-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe documented tier contract is not being validated.
Checking only
response.resultdoes not verify that the simulation succeeded or that the response matches the promised Bronze/Silver/Gold tier shape. Please assert the nested result entry and the decoded tier payload explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/10-Views/get_expert_tier.bru` around lines 32 - 34, The tier response test only checks that response.result exists, so it never validates the documented contract. Update the “Tier data returned” assertion in get_expert_tier.bru to inspect the nested result entry and explicitly verify the decoded tier payload shape and expected Bronze/Silver/Gold values, using the existing response and decoded tier parsing logic in the test block.docs/api/06-Staking/pending_rewards.bru-19-19 (1)
19-19: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the placeholder transaction with a runnable fixture.
"AAAAAgAAAAB4c2VjcmV0...base64-encoded-xdr..."is still a stub, so this request cannot be executed as a ready-to-use Bruno example. The collection needs either a real example XDR or a templated way to build one from the env inputs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/06-Staking/pending_rewards.bru` at line 19, The pending_rewards Bruno request still uses a placeholder transaction value, so update the request fixture to be executable by replacing the stub in the pending rewards example with a real base64 XDR or a templated transaction built from the existing env inputs. Locate the request payload that sets transaction in pending_rewards.bru and make it runnable by wiring it to the same variables and request structure used by the other staking examples, rather than leaving a hardcoded stub.docs/api/06-Staking/stake_unstake.bru-32-38 (1)
32-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the success assertion mandatory.
The
if (response.result)guard makes the core success check optional, so this request can pass without proving the transaction was accepted. Assertresponse.result.hash(and the returned submission status, if applicable) unconditionally instead of skipping the check whenresultis missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/06-Staking/stake_unstake.bru` around lines 32 - 38, The success validation in the staking test is currently optional because `response.result` is guarded by an `if`, so the request can pass without proving acceptance. In `stake_unstake.bru`, make the assertion in the `test("Staking operation accepted")` block unconditional by checking `response.result.hash` directly, and include the returned submission status there as well if that field is part of the response contract; keep the `test("No error")` check separate and unchanged.docs/api/10-Views/get_fee_config.bru-32-39 (1)
32-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssert the fee-config fields, not just that
resultexists.This endpoint documents
first_tier_limit,first_tier_bps, andsecond_tier_bps, but the test never validates that shape. Please check the decoded config payload explicitly so this request catches contract regressions instead of only transport-level failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/10-Views/get_fee_config.bru` around lines 32 - 39, The current test in get_fee_config only checks that response.result exists and that there is no error, so it misses contract shape regressions. Update the Fee config returned assertion to decode the config payload and explicitly verify the documented fee-config fields, including first_tier_limit, first_tier_bps, and second_tier_bps, using the response/result handling already present in the test block.docs/api/10-Views/calculate_platform_fee.bru-32-36 (1)
32-36: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCheck nested simulation errors here.
For
simulateTransaction, contract failures are reported underresponse.result.results[*].error;expect(response).to.not.have.property("error")will miss those. Please validate the nested result entry and its return shape so this request fails on an unsuccessful fee calculation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/10-Views/calculate_platform_fee.bru` around lines 32 - 36, The fee calculation test only checks the top-level response and can miss contract failures reported inside simulateTransaction under response.result.results[*].error. Update the assertion in the calculate_platform_fee.bru scenario to inspect the nested simulation result returned by simulateTransaction and verify the specific entry’s shape, so an unsuccessful fee calculation is caught even when response.error is absent.docs/api/11-Advanced/claim_failed_transfer.bru-27-35 (1)
27-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis request needs a real acceptance check.
Right now the test only proves the RPC call returned a
resultobject. That is not enough forsendTransaction, because non-accepted submissions can still satisfy that shape. Please add the same no-error plus accepted-result assertions used by the stronger transaction specs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/11-Advanced/claim_failed_transfer.bru` around lines 27 - 35, The current test in the claim_failed_transfer spec only checks that a response object exists, which is too weak for sendTransaction validation. Update the tests block to use the stronger acceptance assertions from the other transaction specs: verify there is no error in the response and that the result is explicitly accepted, using the existing response/result handling in this file so the check matches the real RPC acceptance behavior.docs/api/09-Subscriptions/subscription.bru-27-38 (1)
27-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis success test is too shallow for a transaction flow.
expect(response).to.have.property("result")plus no top-levelerrorstill allows unsuccessfulsendTransactionstatuses to pass. Please assert an accepted transaction result shape here as well, otherwise a broken subscription request can still report green in Bruno.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/09-Subscriptions/subscription.bru` around lines 27 - 38, The subscription success test is too shallow and can pass even when the transaction was not actually accepted; update the tests block in subscription.bru to assert the accepted transaction result shape from res.getBody(), not just the presence of result and absence of error. Use the existing response variable and strengthen the “Subscription operation accepted” check so it verifies the sendTransaction outcome indicates acceptance, alongside the current status assertion.docs/api/07-Treasury/withdraw_treasury.bru-27-35 (1)
27-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTighten the success assertion beyond
resultpresence.Line 32 currently passes any JSON-RPC reply that includes
result, so rejectedsendTransactionresponses can still look green here. Match the stronger pattern used indocs/api/04-Session-Lifecycle/start_session.bru:27-39by also asserting the response has no top-levelerrorand checking an accepted transaction shape instead of onlyresult.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/07-Treasury/withdraw_treasury.bru` around lines 27 - 35, The withdrawal test only checks that the JSON-RPC response has a result, which can allow failed sendTransaction replies to pass. Update the tests block in withdraw_treasury.bru, using the same stronger approach as start_session.bru, so the success case also asserts there is no top-level error and verifies an accepted transaction response shape rather than just the presence of result.docs/api/11-Advanced/start_session_with_voucher.bru-27-38 (1)
27-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
jsonrpc === "2.0"does not prove the voucher flow succeeded.This still lacks an assertion on the accepted
sendTransactionresult shape/status, so a rejected submission can pass as long as the RPC envelope is valid. Swap the JSON-RPC version check for a transaction acceptance check here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/11-Advanced/start_session_with_voucher.bru` around lines 27 - 38, The current test in start_session_with_voucher only verifies the JSON-RPC envelope via response.jsonrpc and does not confirm the voucher flow actually accepted the sendTransaction request. Update the test block to assert the accepted sendTransaction result shape/status instead of checking the RPC version, using the existing response object and the voucher-session test cases to locate the assertion.docs/api/08-Fixed-Price/fixed_price_session.bru-27-38 (1)
27-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
sendTransactioncan still fail while these tests pass.The current checks only prove that the RPC envelope is shaped correctly. They do not verify that the transaction was actually accepted, because
response.resultcan exist for non-success statuses too. Add an assertion on the accepted result shape/status here, otherwise this request does not meet the “valid response-shape” requirement from the collection objective.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/08-Fixed-Price/fixed_price_session.bru` around lines 27 - 38, The current tests in fixed_price_session.bru only check that a result exists, which can pass even when sendTransaction fails. Update the tests block to assert the actual accepted response shape/status from res.getBody(), using the existing response variable and the transaction result fields, so the check verifies the request was truly accepted rather than only validating the RPC envelope.docs/api/10-Views/get_session.bru-27-41 (1)
27-41: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCheck the nested simulation error, not just the top-level envelope.
For
simulateTransaction, contract failures surface underresponse.result.results[0]?.error, so Line 38 can still pass on a failed simulation.docs/api/10-Views/get_fee_config.bru:27-40already uses the safer pattern; this file should do the same.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/10-Views/get_session.bru` around lines 27 - 41, The session test only checks the top-level response envelope, so it can miss a nested simulation failure; update the test block in get_session.bru to inspect the simulateTransaction result payload itself. Use the same safer assertion pattern as get_fee_config.bru by verifying the nested response.result.results[0]?.error is absent (or equivalent) rather than only checking response.error, and keep the existing response/result presence checks in sync with the nested structure.docs/api/10-Views/platform_stats.bru-27-38 (1)
27-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThese checks miss contract-level simulation failures.
jsonrpc === "2.0"only validates the RPC wrapper, not the contract response. For view calls, you also need to inspectresponse.result.results[0]?.error; otherwise this request can pass even when the simulated contract invocation failed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/10-Views/platform_stats.bru` around lines 27 - 38, The current checks in the tests block only validate the JSON-RPC wrapper and can miss a failed contract simulation. Update the response assertions in the existing test logic to also inspect response.result.results[0]?.error for the view call, alongside the current status/result/jsonrpc checks, so the contract-level failure is detected even when the RPC envelope looks valid.docs/api/10-Views/voting_power.bru-27-38 (1)
27-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTop-level
erroris the wrong failure check for this view.A Soroban simulation can return HTTP 200 with no top-level JSON-RPC
errorand still carry a contract failure inresponse.result.results[0]?.error. Add that nested assertion here so the voting-power request fails when the contract call does.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/10-Views/voting_power.bru` around lines 27 - 38, The voting power test only checks the top-level JSON-RPC error field, which can miss a contract failure returned inside the simulation result. Update the tests in voting_power.bru to keep the existing status/result checks and add an assertion against response.result.results[0]?.error so the request fails when the Soroban contract call fails. Use the existing response variable in the tests block to locate and verify the nested failure payload.docs/api/11-Advanced/slash_expert.bru-13-35 (1)
13-35: 🎯 Functional Correctness | 🟠 MajorFix the RPC payload and success check
resourceConfigbelongs onsimulateTransaction, notsendTransaction; this request should only send the signedtransactionor switch methods.- Replace the placeholder XDR with a real signed transaction, and don’t call this “Slash executed” unless you follow the returned hash with
getTransaction—response.resultonly shows submission.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/11-Advanced/slash_expert.bru` around lines 13 - 35, The RPC example in slash_expert.bru is using the wrong payload shape for sendTransaction and an inaccurate success assertion. Update the request so sendTransaction only includes the signed transaction, or switch the method to simulateTransaction if resourceConfig is required, and replace the placeholder XDR with a real signed transaction. Also adjust the test in the same snippet so it does not claim “Slash executed” based only on response.result; use a submission-oriented check here, and only verify execution after following the returned hash with getTransaction.docs/api/11-Advanced/initiate_upgrade.bru-13-38 (1)
13-38: 🎯 Functional Correctness | 🟠 MajorUse
simulateTransactionforresourceConfig, and replace the placeholder XDR
resourceConfig.instructionLeewaybelongs onsimulateTransaction, notsendTransaction, so this request body is invalid as written. The XDR is still a placeholder, and if this example is meant to show the upgrade flow end-to-end, add agetTransaction/status check after submitting the tx instead of relying onresponse.resultalone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/11-Advanced/initiate_upgrade.bru` around lines 13 - 38, Move resourceConfig.instructionLeeway off the sendTransaction example and into a simulateTransaction request, since that field belongs on simulation rather than submission. In the initiate_upgrade.bru example, replace the placeholder transaction XDR with a real upgrade transaction payload, and update the tests around response/result to reflect the actual flow. If this doc is intended to demonstrate the full upgrade sequence, use sendTransaction for submission and add a follow-up getTransaction or status verification step instead of checking response.result alone.
🧹 Nitpick comments (1)
docs/api/mainnet.env (1)
5-7: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid committing secret-key fields; document and gitignore real env files.
Even as placeholders,
adminSecretKey/seekerSecretKey/expertSecretKeyin a tracked env file invite users to fill in realS...secret seeds and accidentally commit them. Consider shipping these as a*.env.exampletemplate and gitignoring the working copies, or referencing secrets via a local-only override.Note: the Betterleaks hit on Line 8 (
tokenId) is a false positive — that's a publicC...contract address, not a secret.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/mainnet.env` around lines 5 - 7, The tracked env template includes secret-key fields that could lead to real credentials being committed. Update the `docs/api/mainnet.env` template so `adminSecretKey`, `seekerSecretKey`, and `expertSecretKey` are not shipped as live-looking values; instead move this to an example-only template (for example a `*.env.example`-style file) and ensure the real working env files are gitignored or handled via a local override. Keep `tokenId` unchanged since it is a public contract address, not a secret.Source: Linters/SAST tools
2 participants
Summary
This PR implements two feature requests:
Issue #281 — Bruno API Collection
docs/api/Issue #282 — Soroban TTL Extension & Rent Automation
min_ttl_ledgers(default 100k) andmax_ttl_ledgers(default 2M) storage thresholds instorage.rsextend_persistent_ttlandextend_instance_ttlgeneric helpersregister_expert,create_active_session,save_session(used by settle)initializeand adds admin setters (set_min_ttl_ledgers,set_max_ttl_ledgers) and view functions (get_min_ttl_ledgers,get_max_ttl_ledgers)Closes #281
Closes #282
Summary by CodeRabbit
New Features
Bug Fixes