Echo bidder identity and qualify pre-auction rejections on the auction response - #49
Merged
Conversation
dsabonis
force-pushed
the
echo-attested-bidder-identity
branch
from
August 6, 2026 11:27
083f1c4 to
061273f
Compare
dsabonis
marked this pull request as ready for review
August 6, 2026 11:35
dsabonis
force-pushed
the
echo-attested-bidder-identity
branch
from
August 6, 2026 11:38
061273f to
6505dc9
Compare
dsabonis
force-pushed
the
echo-attested-bidder-identity
branch
from
August 6, 2026 11:49
6505dc9 to
ba1b218
Compare
Bid IDs are only unique per bidder, so a bare ID cannot be attributed back to a seat when two bidders in a round share one — and many DSPs number their bids "1". ExcludedBids and FloorRejectedBidIDs carried IDs only, leaving the host to reconstruct the seat from the ID alone. That routes a rejection's loss notice to a bidder that was never rejected, and lets an ID-wide strip drop another seat's ranked winner from the response. The enclave knows the bidder at both points and was discarding it: ExcludedBid is built from a bid that embeds CoreBid.Bidder, and EnforceBidFloor iterates full CoreBids to return only bid.ID. Every bid reference crossing the boundary now names its bidder. ExcludedBid gains Bidder; FloorRejected and PriceRejected join the response envelope as []core.BidRef. PriceRejected is new on the wire — RunAuction already computed it and ProcessAuction dropped it, so those bidders received no loss notification at all. Additive and backward compatible in both directions. FloorRejectedBidIDs is retained (deprecated) so a host predating the qualified lists keeps working against a newer enclave, and the new fields are omitempty so a newer host sees an unchanged response from an older enclave and falls back to reconstruction. AuctionResult keeps its ID-only slices, derived from the qualified ones, so callers that read them are unaffected. Testing - go build, go vet, go test ./... clean. - New coverage pins the collision the qualified lists exist for: two bidders share a bid ID, only one is below floor, and the rejection names the right seat while the winner keeps the shared ID. Co-authored-by: Cursor <cursoragent@cursor.com>
Marking the ID-only rejection fields deprecated correctly signals downstream hosts to migrate, but it also trips staticcheck on the few in-repo uses that must remain. ProcessAuction still populates FloorRejectedBidIDs so hosts predating the qualified lists keep working, and a test asserts exactly that, so both are annotated with the reason rather than dropping the deprecation marker and losing the signal. The E2EE price-rejection test had no such reason to read the deprecated field, so it now asserts PriceRejected instead — which also pins the bidder, the thing the qualified field exists to carry. Co-authored-by: Cursor <cursoragent@cursor.com>
nickpell
approved these changes
Aug 6, 2026
Member
There was a problem hiding this comment.
I added a commit to make the rejection lists bidder-qualified, to enable correct assignment of the loss reason. Feel free to modify that commit.
Looks ready to merge - 👍 on keeping the bidder names out of the attestation, that's a great way to maintain the current contract along with better management of the bids.
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.
The signed attestation deliberately carries no bidder identity:
AuctionAttestationUserDatais shared with SDKs and bidders, so the winner and runner-up are attested asCoreBidWithoutBidder(bid ID, price, currency, deal ID, media type only). The host then has to reconstruct which bidder each attested bid belongs to — and bid IDs are only unique per bidder, so when IDs collide across bidders in a round the reconstruction is ambiguous.This adds
WinnerBidderandRunnerUpBiddertoEnclaveAuctionResponse, populated from the auction result. They echo theBidderfield the host itself supplied on the request bids, so they reveal nothing to the host that it does not already know, and they are plain envelope fields — not part of the signed COSE user data, which stays bidder-free. Only the winner and runner-up identities are echoed, never the full bid list.Both fields are
omitemptyand additive: hosts that don't read them see an unchanged response, and hosts that do read them treat them as an unsigned hint (the attested bid ID, price, and media type remain the authoritative record).Testing
go test ./...— the ProcessAuction ranking tests now also assert the echoed identities (including under adjustment factors, where the echo must follow the adjusted ranking).