Repository navigation
fix: close the validation gaps and make agent reputation measurable - #27
Merged
Merged
Conversation
Two related holes, both on the path immediately before funds move. tokenPairMatches discarded its proposal argument. The check labelled "token pair matches" verified only that the intent's own fields were non-empty and differed from each other, which no proposal could fail. A proposal swapping an unrelated pair validated clean. Fixing it properly needed the data to compare: Proposal carried no token fields at all, so the pair an agent intended to execute was never recorded. Those fields are added and threaded through the DTO, and an omitted pair is now a failure rather than a pass, so leaving them blank cannot restore the old behaviour. Separately, the validate tags on every request DTO were inert — the module had no validator at all, so an empty proposal was persisted unchallenged. Tags are now enforced at all seven decode sites, with every failure reported at once so a caller fixing a malformed request does not discover the problems one round-trip at a time. Slippage and quality move from `required` to range checks: `required` rejects the zero value, and zero projected slippage is a legitimate claim. Adds the first tests in the module, covering both regressions.
The agents resource was read-only. Proposals require an existing agent
row, but nothing could create one, so a fresh database could not accept
a single proposal — the system was unusable from empty.
Adds POST /agents, GET /agents and PATCH /agents/{id}/status.
Reputation is created alongside the agent rather than lazily, because
the leaderboard skips agents with no reputation row and a newly
registered agent would otherwise be invisible until its first
settlement.
Wallet addresses are unique: a wallet identifies an agent on chain, so
two agents sharing one would make settlement ambiguous.
Status is a pointer to a bool. With a plain bool, "deactivate this
agent" and "leave the status alone" arrive as the same JSON, and the
deactivation would be silently ignored. Agents are deactivated rather
than deleted, since proposals and executions reference them and
removing the row would orphan settled history.
Verified against a live Postgres: creation, duplicate rejection,
listing, the active filter, and both status transitions.
Intents carried no chain. The dApp now routes per chain and connects wallets on both Arc and Stellar, so an intent submitted from the Stellar side was indistinguishable from an Arc one by the time it reached this service, and would settle against the wrong contracts. Adds a chain discriminator, defaulting to Arc when omitted so clients written before multi-chain keep working. Wallet addresses are validated against the chain at the API boundary. The two formats are unmistakable — 20 hex bytes behind 0x, or 56 base32 characters behind G — so accepting a mismatched pair only defers the failure to settlement, which is the most expensive place to find it. The family split (evm vs stellar) is deliberate: code that must branch between the two worlds branches on family rather than chain name, so adding another EVM chain does not mean revisiting every switch. Verified against a live Postgres: both chains persist, a mismatched address is rejected with a message naming the expected format, an unsupported chain is rejected, and an omitted chain still defaults.
Agents self-reported projected_slippage, won competitions on it, and no record was kept of whether the projection held. An agent that promises 0.1% and delivers 2% was indistinguishable from one that promises and delivers 0.1%. Executions now record the projection alongside the actual, plus the delta between them. The projection is copied onto the execution rather than read back through the proposal, so the comparison survives the proposal being edited or soft-deleted. Reputation gains a running mean of that delta. Separately, AvgSlippageDelivered was declared on the model but never written, so the leaderboard has been showing zero for every agent since it was added; both means are now maintained in SQL against the pre-increment count, so a concurrent settlement cannot read a stale average and overwrite it. Also fixes agent reputation rows being created without a primary key. The column has no database-side default, so the first agent inserted an empty string and the second collided with it — found by exercising the endpoint against a real Postgres rather than in the unit tests. Verified end to end: two settlements at 2% and 1% against a promised 0.1% produce an average delivered of 0.015 and an average delta of 0.014.
This was referenced Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #23, #14, #15, #26, #21.
Five issues that turned out to be one thread: the path from "an agent proposes" to "an agent gets paid and earns a reputation" had holes at almost every step.
#23 — proposals were never checked against their intent
tokenPairMatchesdiscarded its proposal argument. The check labelled "token pair matches" verified only that the intent's own fields were non-empty and differed from each other — something no proposal could ever fail. A proposal to swap USDT→WBTC against an intent for USDC→WETH validated clean and went to settlement.Fixing it needed data that did not exist:
Proposalcarried no token fields at all, so the pair an agent intended to execute was never recorded. Those are added and threaded through the DTO. An omitted pair is a failure, not a pass — otherwise leaving the fields blank would restore the original bug.Comparison is case- and whitespace-insensitive: agents submit symbols as free text, and
usdcnaming the same asset asUSDCis a formatting difference, not a mismatch.#14 — the validate tags were decoration
Every request DTO carried
validate:"required"tags and the module had no validator in it at all. An empty proposal — no agent, no venue, negative slippage — was persisted unchallenged.Now enforced at all seven decode sites, reporting every failure at once so a caller fixing a malformed request doesn't discover the problems one round-trip at a time. Field names are reported as the caller sent them (
agent_id, notAgentID).Two tags changed meaning:
ProjectedSlippageandProjectedExecutionQualitymove fromrequiredto range checks, becauserequiredrejects the zero value and zero projected slippage is a legitimate claim.#15 — the system could not be used from empty
Proposals require an existing agent row, and nothing could create one. Adds
POST /agents,GET /agents(with?active=true), andPATCH /agents/{id}/status.Reputation is created alongside the agent, because the leaderboard skips agents with no reputation row — a newly registered agent would otherwise be invisible until its first settlement.
IsActiveis a*bool: with a plain bool, "deactivate this agent" and "leave the status alone" arrive as identical JSON and the deactivation is silently ignored. Agents are deactivated rather than deleted, since executions reference them.#26 — intents did not know which chain they settle on
The dApp routes per chain and connects wallets on both Arc and Stellar, so a Stellar intent was indistinguishable from an Arc one here — and would settle against the wrong contracts.
Wallet addresses are now validated against the chain at the API boundary. The formats are unmistakable (20 hex bytes behind
0x, or 56 base32 characters behindG), so accepting a mismatch only defers the failure to settlement, which is the most expensive place to find it.Omitting the chain defaults to Arc, so clients written before multi-chain keep working.
#21 — reputation measured nothing
Agents self-reported
projected_slippage, won on it, and nothing recorded whether the projection held. An agent promising 0.1% and delivering 2% looked identical to one that promised and delivered 0.1%.Executions now store the projection, the actual, and the delta. Reputation gains a running mean of that delta.
Separately:
AvgSlippageDeliveredwas declared on the model but never written, so the leaderboard has been showing zero for every agent since it was added. Both means are now maintained in SQL against the pre-increment count, so a concurrent settlement cannot read a stale average and overwrite it.A bug found by testing against a real database
AgentReputation.IDis a text primary key with no database-side default. Creating a reputation row without setting it inserted an empty string, so the second agent ever created collided with the first. The unit tests passed; only exercising the endpoint against Postgres surfaced it.Verification
The module had zero test files. This adds 30+ across four packages, covering each regression.
Everything was also driven against a live Postgres:
0.015and avg delta0.014— exactly rightgo build,go vetandgo testall clean.gofmtis back to the six pre-existing unformatted files that were already on main; I formatted only what I touched rather than sweeping unrelated code into the diff.