Fix quote verification for sell=buy where receiver != owner

[MartinquaXD]

Description

Manual testing revealed that the adjustments we make to the quote verification report for buy=sell is too broad. The quote verification only breaks when token flow back into the same account. If the buy tokens go into a different account (e.g. when you set a custom receiver) we don't have to adjust anything. This currently leads to quotes for buy=sell and owner!=receiver being twice as good as they should be.

Changes

Only apply the adjustment if owner == receiver
some minor adjustments to the comment explaining the logic
moved ApproxEq code into the numbers crate to reuse it in the e2e test

How to test

extended an existing e2e test for sell=buy asserting that quotes for owner==receiver and owner!=receiver are within 1 basis point from each other

[gemini-code-assist]

Code Review

This pull request correctly fixes a bug in the quote verification logic for sell=buy trades where the receiver is not the owner. The adjustment is now correctly applied only when the owner and receiver are the same. The accompanying refactoring of the ApproxEq trait into the number crate is a good improvement for code reuse. The new e2e test effectively validates the fix. The suggestion to improve the precision of the assertion in the new test is valid and should be considered.

[jmg-duarte]

LGTM, left a question

[jmg-duarte]

Should we gate this file behind a "test" feature?

[MartinquaXD]

I mean we can do that but so far I always felt that feature gating this stuff is more of a headache than it actually resolve (i.e. compile times, preventing to call this function outside of tests).

[MartinquaXD]

I tried feature gating this code but what I really don't like is the discoverability of things. If a crate already depends on number and you know that is_approx_eq is a thing it's not possible to just type that and rely on rust-analyzer to correctly import that.

My gut feeling is to not feature gate things unless this causes significant compile time issues. And in our case the code will almost always require tests to be built since you need to test the change.