fix: validate token precision multiplier

[bayological]

Description

This change adds validation to the function BiPoolManager.setTokenPrecisionMultipliers similar to that in the create pool function. The change verifies the tokens being added to the mapping are known by the reserve and the precisions are within the allowed range.

Other changes

• Added an event to be emitted when the multipliers are updated

Tested

• Unit tests were added to verify the change functions as expected.

Related issues

• Closes [M-1] setTokenPrecisionMultipliers allows arbitrary decimal multipliers #238

Backwards compatibility

• N/A

Documentation

• N/A

[philbow61]

LGTM if we want to go down this route 👍
I personally would argue there is no need for the setTokenPrecisionMultipliers function in the beginning.
They are automatically set in the createExchange function by calling decimals() on the token contract. The only context where these multipliers are used is in quotes/swaps and those only work on a valid exchanges. So what is the use case for this function in my opinion it can only cause problems by setting wrong values, but maybe I am missing something 😁

[bayological]

LGTM if we want to go down this route 👍 I personally would argue there is no need for the setTokenPrecisionMultipliers function in the beginning. They are automatically set in the createExchange function by calling decimals() on the token contract. The only context where these multipliers are used is in quotes/swaps and those only work on a valid exchanges. So what is the use case for this function in my opinion it can only cause problems by setting wrong values, but maybe I am missing something grin

I agree with you. I don't feel strongly about keeping this function, it does not add much. However, there is no harm in having it there 🤷‍♂️. The only use case that comes to mind is if the multiplier was set incorrectly when creating an exchange, this would allow that to be corrected without recreating the exchange.

[nvtaveras]

Agree with @philipraetsch here that this function seems a bit pointless.

The only use case that comes to mind is if the multiplier was set incorrectly when creating an exchange, this would allow that to be corrected without recreating the exchange.

The value added doesn't seem worth it to me. In the spirit of having less code, I would say let's remove it

---

Update: chatted with the big boss offline and he said: "kill it with fire" 🔥

[philbow61]

🔥