chore(sol): default decimal adjustment to 1

[0xmichalis]

Currently the pool contract is converting the underlying decimals to the pool token decimals before calling the fee calculator so there is no need to adjust the amount in the fee calculator, hence default feeToUnderlyingDecimalsScale to 1.

[0xmichalis]

We should also consider reverting #55 and treat amounts sent to the fee calculator as pool amounts. Anyway users shouldn't need to interact directly with the fee calculator, instead the pool contract offers a holistic interface that handles conversions.

[0xmichalis]

If we revert #55 at the very least we should document that callers should be converting the provided amounts to the pool token's precision in NatSpec.

We should also consider reverting #55 and treat amounts sent to the fee calculator as pool amounts. Anyway users shouldn't need to interact directly with the fee calculator, instead the pool contract offers a holistic interface that handles conversions.

A few arguments against reverting:

• the amounts provided to the fee calculator are underlying token amounts so it's clearer to allow accepting amounts in their native precision. Without deep knowledge of all the pool contracts, I can see how the conversion happening in the pool and keeping the fee calculator oblivious to the underlying's decimals can be confusing and harder to debug
• with the extra functionality the calculator can be reused by other protocols without the need to force those protocols to do their own conversions before having to use the calculator

[lirona]

can you please add "distributed" to this line while you're at it? it's hurting my eyes :))

[lirona]

how are no tests affected by this change?

[0xmichalis]

can you please add "distributed" to this line while you're at it? it's hurting my eyes :))

Feel free to open a PR for it please as it's not related to my changes.

[0xmichalis]

how are no tests affected by this change?

I think this is because the fuzzing test suite was already setting the underlying decimals to 1 here.

[sirnicolaz]

@0xmichalis i in principle also don't have a strong opinion, but defaulting to 1 seems to be a half baked solution between delegating the conversion to the Pool vs having the calculator taking this responsibility.

My preferred way to go (assuming it's THE WAY) would be to either remove the knowledge of the underlying decimals completely from the calculator, or leave it all there. But like it is here, it's a half step, which if left untouched will leave even more confusion around.

Do you think we can just roll back #55 at this point?

[0xmichalis]

My preferred way to go (assuming it's THE WAY) would be to either remove the knowledge of the underlying decimals completely from the calculator, or leave it all there. But like it is here, it's a half step, which if left untouched will leave even more confusion around.

Not sure what you mean exactly by "leave it all there"? Presumably you mean that the pool should stop sending converted amounts to the fee calculator and the fee calculator should be configured to work with the underlying decimals? Agreed that as it stands it's half-baked, it's likely fine to revert #55 at the expense of making the calculator less reusable for ERC-1155 use cases.

[0xmichalis]

Closing in favor of #58

[sirnicolaz]

Not sure what you mean exactly by "leave it all there"? Presumably you mean that the pool should stop sending converted amounts to the fee calculator and the fee calculator should be configured to work with the underlying decimals?

that's what I meant.

it's likely fine to revert #55 at the expense of making the calculator less reusable for ERC-1155 use cases

true, but we can still roll out a new calculator in the future if we see the need/demand, and make it more flexible in case...