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

Flip the sign on currencyDeltas and accountDelta #467

Closed
danrobinson opened this issue Jan 29, 2024 · 0 comments · Fixed by #481
Closed

Flip the sign on currencyDeltas and accountDelta #467

danrobinson opened this issue Jan 29, 2024 · 0 comments · Fixed by #481
Assignees
Labels

Comments

@danrobinson
Copy link
Contributor

danrobinson commented Jan 29, 2024

Component

Delta accounting

Describe the suggested feature and problem it solves.

This is a change to conventions to make singleton accounting more consistent and intuitive.

The singleton tracks transient balances for users in a currencyDelta mapping. It tracks changes to those transient balances using the BalanceDelta type.

Right now, these are all signed integers, reflecting the fact that unlike ERC-20 or ERC-6909 balances, transient balances and deltas can be temporarily negative as long as they are resolved at the end of the transaction.

But separately from this, currencyDelta also flips the usual sign for tracking balances. If the user deposits more money into the singleton, or if some pool or other account transfers money to the user, the user's transient balance on the singleton goes _down. When the user owes the pool money, they have a positive currencyDelta on the pool.

This is inconsistent with how the singleton accounts for ERC-6909 balances (resulting in the odd interaction that when a user converts transient balance into ERC-6909 balance using mint, both of its balances go up). It is also inconsistent with how essentially all ERC-20 contracts track balances of users (where a positive balance means the user has money rather than owes money). Finally, it is counterintuitive.

Since BalanceDeltas are directly applied to currencyDelta, they also follow these flipped conventions—functions return negative numbers for BalanceDeltas when a user should receive money as a result of the function, and positive numbers when a user should pay money as a result of the function.

Describe the desired implementation.

Flip the meaning of currencyDelta and balanceDelta, and flip the sign on every "delta" at the moment it is first created.

Describe alternatives.

No response

Additional context.

One reason to consider keeping the current convention is that the convention for positive deltas meaning the user owes money to the pool was set by the callbacks in Uniswap v3 (such as https://github.com/Uniswap/v3-core/blob/main/contracts/UniswapV3Pool.sol#L482).

However, I think that was a very different context and different functionality. Uniswap v3 did not have a full accounting system for user deltas, so it was freer to make a different choice.

One open question is whether we should also change the convention for the amountSpecified in swap to match this (so, for example, a user passes in -100 when they want to pay in exactly 100 of token0, and 100 when they want to receive exactly 100 of token1). I could see both sides of it.

@hensha256 hensha256 linked a pull request Mar 5, 2024 that will close this issue
@hensha256 hensha256 self-assigned this Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants