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

Change approvedSellers to sellerCapabilities #403

Merged
merged 5 commits into from
Feb 6, 2023
Merged

Conversation

caraitto
Copy link
Collaborator

sellerCapabilities is more granular and allows only granting permissions for latency stats, or interest group counts.

It can also be extended in the future with additional capabilities.

sellerCapabilities is more granular and allows only granting permissions for latency stats, or interest group counts.

It can also be extended in the future with additional capabilities.
@dmdabbs
Copy link
Contributor

dmdabbs commented Nov 30, 2022

interestGroupCount: The number of the interest groups which could participate in the auction (i.e. the interest group’s owner was listed in the interestGroupBuyers). This requires the interestGroupCounts sellerCapabilities permission.

The buyer-specified perBuyerGroupLimits may constrain participation. Will this report the IG's count or what may have been limited by perBuyerGroupLimits?

@caraitto
Copy link
Collaborator Author

caraitto commented Dec 1, 2022

@dmdabbs IIUC, the question is interestGroupCount should be the number of interest groups, or min(perBuyerGroupLimits, number of interest groups)?

The auction config sets perBuyerGroupLimits, so presumably the seller already knows this value (although the runAdAuction() call may be made from a frame whose origin isn't the seller's -- but this is generally done in partnership with the seller). So, it seems like reporting the unconstrained number of interest groups makes more sense? Alternatively, both could be reported, if there's value in reporting both (they would both be controlled by the interestGroupCounts sellerCapabilities permission, just like bidCount).

@dmdabbs
Copy link
Contributor

dmdabbs commented Dec 1, 2022

So, it seems like reporting the unconstrained number of interest groups makes more sense?

Thanks @caraitto. Yes, I agree. I asked because number of the interest groups which could participate was ambiguous.

@caraitto
Copy link
Collaborator Author

caraitto commented Dec 1, 2022

So, it seems like reporting the unconstrained number of interest groups makes more sense?

Thanks @caraitto. Yes, I agree. I asked because number of the interest groups which could participate was ambiguous.

Got it -- I updated the text to make this more clear.

@dmdabbs
Copy link
Contributor

dmdabbs commented Dec 1, 2022

The interestGroup provided to navigator.joinAdInterestGroup() will contain a new field named sellerCapabilities, a dict keyed by seller origin...

Seller/publishers will apreciate these stats, and buyers this control over which sellers may receive them. However, even if only a small circle of ad techs ends up as FLEDGE sellers (unlikely), all a buyer's joinAdInterestGroup() registrations will be burdened with this weight. As it is there are calls to raise the 50kB interest group size limit.

Perhaps consider some buyer origin .well-known uri from which to fetch and cache this permission info.

@caraitto
Copy link
Collaborator Author

caraitto commented Dec 1, 2022

The interestGroup provided to navigator.joinAdInterestGroup() will contain a new field named sellerCapabilities, a dict keyed by seller origin...

Seller/publishers will apreciate these stats, and buyers this control over which sellers may receive them. However, even if only a small circle of ad techs ends up as FLEDGE sellers (unlikely), all a buyer's joinAdInterestGroup() registrations will be burdened with this weight. As it is there are calls to raise the 50kB interest group size limit.

Perhaps consider some buyer origin .well-known uri from which to fetch and cache this permission info.

Yes, this feature will require storage for each of the origin strings. Additionally, the permissions themselves will be just a 32-bit bitfield, so 4 bytes per origin, plus 4 bytes for the all-buyers bitfield.

It may be possible to increase the 50kb limit, but we need to ensure the that the total storage usage for all of FLEDGE is workable, especially on low-end phones that don't have much storage. With a 50kb limit, and 1000 interest groups, that's 50mb of storage used just for FLEDGE interest groups, for instance (with a bit more for k-anon data, sqlite indices, etc.). Let's continue that discussion on #402.

@dmdabbs
Copy link
Contributor

dmdabbs commented Dec 2, 2022

Yes, thank you. Until seeing your example I'd glossed over the "list of strings" bit. It is acceptable to omit the https:// and the browser will supply?

@caraitto
Copy link
Collaborator Author

caraitto commented Dec 5, 2022

Yes, thank you. Until seeing your example I'd glossed over the "list of strings" bit. It is acceptable to omit the https:// and the browser will supply?

I think it's better to have the origin be explicit, to avoid confusion?

@dmdabbs
Copy link
Contributor

dmdabbs commented Dec 5, 2022

I agree. AFAIK, everywhere else buyers/sellers are specified as a URL. I asked based on Paul's suggested example. Perhaps he omitted the protocol as shorthand.

aarongable pushed a commit to chromium/chromium that referenced this pull request Dec 7, 2022
This field controls the behavior of the interest group when bidding for
various sellers (with a * wildcard indicating the default capability
bits). In this CL, add 2 capability bits:

interestGroupCounts: The number of interest groups (unconstrainted by
perBuyerGroupLimits) for this bidder, and the number of bids.

latencyStats: the total generateBid() latency, and the total time spent
fetching trusted bidding signals.

This CL only adds support for parsing the new field and passing it
over Mojo.

Described in
WICG/turtledove#403

Bug: 1186444,1385549
Change-Id: I6ba68022a945edcbf9f6cd8e15d24267235edfcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4040362
Reviewed-by: Qingxin Wu <qingxinwu@google.com>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Cr-Commit-Position: refs/heads/main@{#1080423}
@caraitto
Copy link
Collaborator Author

I added a clarification for what gets reported when no trusted signals were requested by the interest group.

@JensenPaul JensenPaul merged commit 6c972c1 into WICG:main Feb 6, 2023
@caraitto caraitto deleted the patch-3 branch February 6, 2023 21:27
github-actions bot added a commit that referenced this pull request Feb 6, 2023
SHA: 6c972c1
Reason: push, by JensenPaul

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

None yet

3 participants