Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update explainer to include previously discussed multi-SSP mechanism #251

Merged
merged 30 commits into from
Mar 3, 2022

Conversation

MattMenke2
Copy link
Contributor

This adds a multi-SSP mechanism along the lines discussed in #59, #73, and #202.

Some specific design decisions that I'd appreciate feedback on, if folks think they should be changed:

  • Naming. I've gone with "component auctions" for the nested auctions, and "top-level" for the other auction. If folks have other ideas, or think having both "component render urls" and "component auctions" are confusing, suggestions are welcome.

  • This only allows auctions 1-deep, to try and keep things (somewhat) simple. Is there a need for more deeply nested sellers?

  • For the most part, the fields of each configuration are only passed to bidders and the seller worklet in that auction (e.g., auctionSignals in the top-level auction are not passed to seller or buyer worklets in component auctions). We could have a variant of those fields that are global, or have maps that are also keyed on seller to pass data to particular component auctions, but that does potentially introduce a lot of complexity, when fields in the component auctions could be updated directly instead.

  • In the new GenerateBid()->component auction seller ScodeAd()->top-level seller ScoreAd() flow, GenerateBid()'s render URLs and bid are passed directly to both ScoreAd() methods, while only the component seller's metadata is passed to the top-level seller's ScoreAd() method.

  • The top-level ReportResult() return value is passed to the winning bidder's ReportWin() method, but not its component auction's seller worklet's ReportResult() method. Should it be?

FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
Copy link

@sbelov sbelov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
@JoelPM
Copy link
Contributor

JoelPM commented Jan 28, 2022

The top-level ReportResult() return value is passed to the winning bidder's ReportWin() method, but not its component auction's seller worklet's ReportResult() method. Should it be?

I think so? It's possible I haven't fully grokked the changes being discussed here, but if the component auction runner (eg the SSP) doesn't get the result, how will they able to reconcile reporting/discrepancies with the DSP?

@MattMenke2
Copy link
Contributor Author

The top-level ReportResult() return value is passed to the winning bidder's ReportWin() method, but not its component auction's seller worklet's ReportResult() method. Should it be?

I think so? It's possible I haven't fully grokked the changes being discussed here, but if the component auction runner (eg the SSP) doesn't get the result, how will they able to reconcile reporting/discrepancies with the DSP?

It's already told about the result of the auction (winning bidder, price, its desirability score). The only thing it isn't passed it the output of the top-level seller's ReportResult method. It's unclear if that information should be passed to the component seller's ReportResult method, the bidder's ReportWin method, or both. Or we could have separate values for each, for that matter.

@MattMenke2
Copy link
Contributor Author

It's already told about the result of the auction (winning bidder, price, its desirability score). The only thing it isn't passed it the output of the top-level seller's ReportResult method. It's unclear if that information should be passed to the component seller's ReportResult method, the bidder's ReportWin method, or both. Or we could have separate values for each, for that matter.

It would probably be most consistent with how the generate bid / scoring phase are currently structured with respect to the "ad" field to pass the top-level seller's reportResult() output to the component seller's reportResult() method only, and the component seller's reportResult() output only to the reportWin() method of the seller. Of course, we could rework how the "ad" field works as well, to also pass the bidder's data to the top-level seller.

I've gone ahead and moved "topLevelSellerSignals" from being a parameter to only the bidder's reportWin() method to only being a parameter to the component seller's reportResult() method. I'm fine with adding it to both, if folks prefer it. With this approach, it's up to the component seller whether any of the data should be forwarded on to the bidder.

@jeffkaufman
Copy link
Contributor

As discussed in today's call, it sounds like https://github.com/WICG/turtledove/blob/main/Fenced_Frames_Ads_Reporting.md will need to be updated a little given this change. It currently has Destination type: List of values to determine whether this event needs to be reported to both buyer and seller, only buyer or only seller, but it sounds like now there is a third value of "component seller"?

@shivanigithub

aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 3, 2022
This CL adds an AuctionAdConfig array to InterestGroup struct. This will
be used to run multi-seller / component FLEDGE auctions, per
WICG/turtledove#251.

This CL doesn't actually wire up those auctions, it just adds the field
to IDL and Mojo files, and adds code to convert the IDL to Mojo, and
validate the fields.

Bug: 1288865
Change-Id: I717887220b0cf207fc25976ebf8f727cf3ac1ecf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3410871
Reviewed-by: Caleb Raitto <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Matt Menke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#966504}
@aprokofg
Copy link

aprokofg commented Feb 3, 2022

@MattMenke2 Based on the explainer it looks like perBuyerSignals and perBuyerTimeouts could theoretically be specified as part of top-level and component level auctions. Should these perBuyerSignals / perBuyerTimeouts be allowed at top-level auction? If yes, what effect would they have at component-level auctions?

  • Ignored?
  • Merged?
  • Take precedence?

@MattMenke2
Copy link
Contributor Author

@MattMenke2 Based on the explainer it looks like perBuyerSignals and perBuyerTimeouts could theoretically be specified as part of top-level and component level auctions. Should these perBuyerSignals / perBuyerTimeouts be allowed at top-level auction? If yes, what effect would they have at component-level auctions?

  • Ignored?
  • Merged?
  • Take precedence?

Currently, the intention is that each AuctionConfig only applies to the worklets that run as a part of that auction, so the top-level is ignored (as are all parameters - auctionSignals, perBuyerSignals, etc).

So for component bids, we have:

  • bidder generateBid() -> values from component AuctionConfig apply only (auctionSignals, perBuyerSignals, perBuyerTimeouts).
  • component seller scoreAd() -> values from its component AuctionConfig apply only (auctionSignals, sellerSignals, sellerTimeout), and the AuctionConfig passed to it is the component auction AuctionConfig.
  • top-level seller scoreAd() -> values from the top-level AuctionConfig apply only, but the AuctionConfig passed to it is the AuctionConfig for the entire auction.

We could change that if there's consensus around changing behavior, but my thinking this more isolated view of the world reduces the chance of leaking data from the top-level auction to the component auctions, in cases where that's not desired, though it does add some complexity and overhead in the case where the intention is that signals should be merged.

We could consider making heritable duplicate values in the top-level auction with some prefix or suffix, and either pass them in via new parameter, or use them if there's no value in the lower layer auctions, though that does add a bit of complexity.

@MattMenke2
Copy link
Contributor Author

  • top-level seller scoreAd() -> values from the top-level AuctionConfig apply only, but the AuctionConfig passed to it is the AuctionConfig for the entire auction.

To clarify, I mean it has the top-level AuctionConfig, with all component AuctionConfigs also present.

@jeffkaufman
Copy link
Contributor

FWIW "each AuctionConfig only applies to the worklets that run as a part of that auction" sounds very reasonable to me

@MattMenke2
Copy link
Contributor Author

Just a heads up to anyone following at home that I've also added a parameter needed by worklets to indicate that they're willing to participate in component auctions, so that buyers and sellers don't find themselves unexpectedly taking part in auctions they weren't designed for.

This required making the top-level seller scoreAd() scripts return an object as well, when scoring bids from a component auction.

I think we should probably switch to letting scoreAd() return an object in all cases, with just one-element. Perhaps we should get rid of supporting non-object return values, though that would require all experimental scripts be updated, unfortunately.

@MattMenke2
Copy link
Contributor Author

Think it's about time to land this. It's 95% implemented by this point, so a bit late to make changes. I've done a proof-reading pass and updated it to be on top of the latest version of the explainer.

@michaelkleber
Copy link
Collaborator

Looks great, thank you Matt.

@michaelkleber michaelkleber merged commit 8622a63 into WICG:main Mar 3, 2022
@MattMenke2 MattMenke2 deleted the MattMenke2-componentAuctions branch June 15, 2022 00:23
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This CL adds an AuctionAdConfig array to InterestGroup struct. This will
be used to run multi-seller / component FLEDGE auctions, per
WICG/turtledove#251.

This CL doesn't actually wire up those auctions, it just adds the field
to IDL and Mojo files, and adds code to convert the IDL to Mojo, and
validate the fields.

Bug: 1288865
Change-Id: I717887220b0cf207fc25976ebf8f727cf3ac1ecf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3410871
Reviewed-by: Caleb Raitto <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Matt Menke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#966504}
NOKEYCHECK=True
GitOrigin-RevId: bcc9b329a586ca5158653523df2c828e716fb7c3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants