I'd rather not cast them to floats to prevent losing precision on the rate. Is there an issue on using strings?
I can update them on my end, just let me know if you think we might have an issue with that change.

hey, sure, whatever works. the only potential problem is that DB always stores floats, so breaking the standard will be confusing. would it make sense to change other values to keep evm scaling as well? eg: price rates from rate providers?

I'd say we should change all rates to strings 👍
We're doing our best to get balancer-maths and SOR to 100% match against on-chain results, so if we're able to avoid losing precision, I'd rather go that way.
Is there any downside to storing strings other than having to migrate everything?

i'd expect the same goes for token balances? totally agree on data correctness, though the decision, made ages ago, runs deep, impacting things like the subgraph. SOR is a pretty good reason to have evm values. I think we can start adding native values without much or tiny impact on existing code. tagging @franzns for thoughts.

Yeah, we could have a separate issue to keep track of this refactor, but I'd consider it low priority at the moment. Anyway, my suggestion is to already make new ones string/evm scale and even refactor as we touch them on other PRs, rather than keep creating floats and refactor everything at once later.
But that's my opinion.
Totally fine if you guys disagree and prefer to keep the standard (floats) for now and refactor all at once in a single PR.

Just so we have it registered here as well -> I don't think we need to store amounts in evm scale, I just think we need to avoid floats and store human scale as strings.

doesn't convertion from evm to human scale loose precision already?

not necessarily - as long as we store as strings it should be ok - for example:

• evm scale (18 decimals): 12345678901234567890
• human scale (string): 12.345678901234567890
• human scale (float): 12.34567890123457

Ps: I don't remember exactly how many decimals float support, but it will eventually truncate

btw - parseFloat does lose precision, but that's why we shouldn't use it 🙃
we should stick with formatUnits, formatEther, parseUnits and parseEther to manipulate these values

yes, exactly. good summary, thanks!

@johngrantuk - I'd like to discuss this line with you.
I decided to use 1e18 as input to convertToAssets so we always get the equivalent of 18 decimals unwrapRate disregard of the original token decimals (unless I'm missing something 🤔).
This should make it easier to handle any math regarding rates, right?
I know you took a different approach on Paraswap PR, so it would be nice to hear your opinion.

If you do it that way you will then have to scale the rate to the raw decimals when passing it as rate for a Buffer pool. That way the maths will work.

@brunoguerios, some useful test info:
The Gnosis pool, 0x272d6be442e30d7c87390edeb9b96f1e84cecd8d uses a rate provider that is nested. So unwrap rate does not equal rate between aave wsteth and eth. This particular case the rate provider accounts for growth of wsteth in terms of weth and the additional aave yield. This highlighted that rateProvider can not be used for buffer wrap/unwrap which instead should use erc4626 rate.

Mainnet Pool, 0x5dd88b3aa3143173eb26552923922bdf33f50949 has an ERC4626 token with 18 decimals that uses a 6 decimal underlying.
Note for maths: Instead of manually adding support for each ERC4626 implementation (e.g. stata with Ray maths) we always use an 18 decimal scaled rate and do 18 decimal maths to convert. We may end up loosing 100% accuracy but thats deemed acceptable.

So, let me check if I got this right:

1. balancer maths expect rate to be passed in with decimals equals to the erc4626 (wrapped) token
2. internally balancer maths always scale that rate to 18 decimals (possibly losing precision on RAY math implementations)

Ps: thanks for the test info - I'll create some tests to make sure the SOR is working as expected

@brunoguerios - sorry I read this wrong! Always using 1e18 for convertToAssets is correct. You should just be able to pass the returned rate as the buffer rate to the maths and it will work.

This would be so much easier if everyone kept consistency between wrapped/underlying decimals. This one case of 18 decimals wrapped and 6 decimals underlying will require quite a big change 🙄

This feels a bit odd, so it would be nice to get feedback on how to upsert token data

true, it could be be shorter: