From review of #467 (comment r3616741539). Verified against the end-of-train tip (feat/m5-git-tag-pins-umbrella-patch, 24c4d9e).
Both submit outcomes hand back an order UID that nothing ever reconciles against what the orderbook actually stored.
On Posted::AlreadyHeld the UID is derived purely client-side from assembly::order_uid(config.chain, &order, owner), because the API's already-held reply carries no UID by design. On Posted::Accepted the UID is the server's, but nothing asserts the stored order matches the body just submitted. Either way a drift between our UID derivation and the orderbook's would go unnoticed and be trusted downstream as the receipt.
Correction to the review's suggested fix
The review proposes having status_with hard-fail when the fetched order's sell/buy token or validTo disagree with the submitted body. That cannot be done locally: status_with(fetch, config, receipt) only receives the 56-byte receipt, and it deliberately deserializes a single field.
/// The one server field the lifecycle projection reads.
#[derive(Deserialize)]
struct OrderStatusView {
status: OrderStatus,
}So the check needs the expected order threaded to wherever the comparison happens. That is an interface change, not a guard bolted onto the existing read.
Options
Verify at submit time. After post_order returns, GET the UID once and compare the stored order's sellToken, buyToken, sellAmount, buyAmount and validTo against the submitted body. Costs one extra round trip per submission, and confines the change to submit_with.
Verify at status time. Widen OrderStatusView and give status_with the expected order. Free on the request budget, since the status poll already happens, but it widens the adapter's status signature and every caller of it.
Assert the derivation only. Compare the server's returned UID against assembly::order_uid(...) on the Accepted branch, where both are available. This catches exactly the derivation-drift case the review is worried about, costs nothing, and needs no interface change. It does not cover AlreadyHeld, where no server UID exists.
The third is the cheapest way to get the drift alarm and is worth doing regardless of whether either fuller check lands.
Acceptance criteria
A mismatch between the locally derived UID and the orderbook's own record surfaces as a loud typed failure rather than being trusted as the receipt.
The already-held path states in its rustdoc that its UID is locally derived and unverified, if it stays that way.
Both submit outcomes hand back an order UID that nothing ever reconciles against what the orderbook actually stored.
On
Posted::AlreadyHeldthe UID is derived purely client-side fromassembly::order_uid(config.chain, &order, owner), because the API's already-held reply carries no UID by design. OnPosted::Acceptedthe UID is the server's, but nothing asserts the stored order matches the body just submitted. Either way a drift between our UID derivation and the orderbook's would go unnoticed and be trusted downstream as the receipt.Correction to the review's suggested fix
The review proposes having
status_withhard-fail when the fetched order's sell/buy token orvalidTodisagree with the submitted body. That cannot be done locally:status_with(fetch, config, receipt)only receives the 56-byte receipt, and it deliberately deserializes a single field.So the check needs the expected order threaded to wherever the comparison happens. That is an interface change, not a guard bolted onto the existing read.
Options
Verify at submit time. After
post_orderreturns, GET the UID once and compare the stored order'ssellToken,buyToken,sellAmount,buyAmountandvalidToagainst the submitted body. Costs one extra round trip per submission, and confines the change tosubmit_with.Verify at status time. Widen
OrderStatusViewand givestatus_withthe expected order. Free on the request budget, since the status poll already happens, but it widens the adapter's status signature and every caller of it.Assert the derivation only. Compare the server's returned UID against
assembly::order_uid(...)on theAcceptedbranch, where both are available. This catches exactly the derivation-drift case the review is worried about, costs nothing, and needs no interface change. It does not coverAlreadyHeld, where no server UID exists.The third is the cheapest way to get the drift alarm and is worth doing regardless of whether either fuller check lands.
Acceptance criteria
A mismatch between the locally derived UID and the orderbook's own record surfaces as a loud typed failure rather than being trusted as the receipt.
The already-held path states in its rustdoc that its UID is locally derived and unverified, if it stays that way.