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

Explainer changes for how currency-related things work #571

Merged
merged 20 commits into from
Nov 3, 2023

Conversation

morlovich
Copy link
Collaborator

No description provided.

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_extended_PA_reporting.md Outdated Show resolved Hide resolved
Proposed_First_FLEDGE_OT_Details.md Outdated Show resolved Hide resolved
@trentunderwood
Copy link

In this mode (seller currency enabled), all APIs like event reporting, debug event-level loss reporting, and private aggregation will see the converted (bid) values

Question regarding this point: Why can't the seller receive the exact bid in its original currency in reportResult()? It is important that the seller can invoice the buyer the exact amount that was returned from generateBid(). Additionally, it introduces additional risks to the buyer. For instance, what if the seller uses an unfavorable conversion rate? The buyer cannot know about this in advance and won't be able to do anything about it.

Can we change this behavior to allow reportResult() to receive the exact original bid value in its original currency?

@morlovich
Copy link
Collaborator Author

Thanks for the feedback; that's a good point (though beware of rounding rules!); I am working on changing it (well, almost ready to mail to the -code- CL), but one bit of warning: the behavior will be different between M114/115 and 116+.

@michaelkleber
Copy link
Collaborator

Hey @trentunderwood Please do note that the values passed into the reporting functions are rounded, so that they only have 8 bits of mantissa. Please see #486. The work going on here is about currency; the rounding of bid values for privacy reasons is separate.

@trentunderwood
Copy link

Thank you!

I am working on changing it

Do you mean that you are changing the bid passed to reportResult to be in the same currency that was returned from generateBid? Even if the bid is fuzzed a bit, it would still be very helpful for generateBid and reportResult to use the same currency.

@morlovich
Copy link
Collaborator Author

Thank you!

I am working on changing it

Do you mean that you are changing the bid passed to reportResult to be in the same currency that was returned from generateBid? Even if the bid is fuzzed a bit, it would still be very helpful for generateBid and reportResult to use the same currency.

Yep. See https://chromium-review.googlesource.com/c/chromium/src/+/4573913 as well as changes to this pull request.
Note that this is only about reportResult itself, things like private aggregation keyed off reportResult still get things in the converted currency.

@trentunderwood
Copy link

That sounds great - thank you!

@JensenPaul JensenPaul added the Non-breaking Feature Request Feature request for functionality unlikely to break backwards compatibility label Jun 23, 2023
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_extended_PA_reporting.md Outdated Show resolved Hide resolved
Proposed_First_FLEDGE_OT_Details.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@JensenPaul JensenPaul left a comment

Choose a reason for hiding this comment

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

I'm still somewhat confused, so I added some clarification rewordings and asked some verifying questions to make sure I'm understanding this properly.

FLEDGE.md Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated Show resolved Hide resolved
Proposed_First_FLEDGE_OT_Details.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
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
@JensenPaul JensenPaul merged commit c6491c4 into WICG:main Nov 3, 2023
2 checks passed
github-actions bot added a commit that referenced this pull request Nov 3, 2023
SHA: c6491c4
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
Non-breaking Feature Request Feature request for functionality unlikely to break backwards compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants