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

function balanceOfLP returns LPData and not balance in ButteredBread #118

Open
lirona opened this issue Dec 18, 2024 · 7 comments
Open

function balanceOfLP returns LPData and not balance in ButteredBread #118

lirona opened this issue Dec 18, 2024 · 7 comments
Assignees

Comments

@lirona
Copy link
Contributor

lirona commented Dec 18, 2024

the function name and natspec say it returns the balance but it returns LPData which contains balance and scaling factor.
there is another getter called accountToLPBalance which returns the balance.
consider removing this function if only the balance is needed, or changing it's name. for now i'll change the natspec at least :)

@RonTuretzky
Copy link
Collaborator

@secbajor @subject026 do you know the balanceOfLP is used in the crowdstaking app?

@subject026
Copy link
Member

Not right now

@RonTuretzky
Copy link
Collaborator

Not right now

Is it planned in any way? Otherwise we might rename it

@subject026
Copy link
Member

We will need to be able to fetch the scaling factors although I think being able to fetch them on their own rather than paired with a LP balance would be more flexible

@RonTuretzky
Copy link
Collaborator

We will need to be able to fetch the scaling factors although I think being able to fetch them on their own rather than paired with a LP balance would be more flexible

Yea we have another method that exposes it per lp. Great so we can remove this method I think @lirona ! Unless it's used in tests somewhere 🤔

@lirona
Copy link
Contributor Author

lirona commented Dec 24, 2024

hmm i don't see a getter for scaling factors per user anywhere.. @subject026 did you mean you need them per user and lp or just per lp?

@RonTuretzky
Copy link
Collaborator

I believe there's no reason we would need the modifier on the frontend at all, as there's another method that exposes the voting power and the modifier is accounted for in there

RonTuretzky pushed a commit that referenced this issue Dec 27, 2024
* add amount 0 checks (issue #114)

* check transfer return value (issue #115)

* move transfer to the end of _deposit (issue #116)

also moved `_syncDelegation` just for consistency

* fix natspec for balanceOfLP (issue #118)

* _syncVotingWeight in deposit (issue #119)

* add warning to modifyAllowList (issue #117)

* custom errors

* add comment about ButteredBread changes

* revert move transferFrom

* add reentrancy guard on deposit

also added __ERC20Votes_init which does nothing but might change in future versions

* add nonReentrant on withdraw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants