Skip to content

fix: allow full curvejs reinitialization #465

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

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Conversation

DanielSchiavini
Copy link
Contributor

@DanielSchiavini DanielSchiavini commented May 8, 2025

  • Wrap the default exported curve instance in a method. Add a named export this method. Do the same for the Curve class.
  • Export a default instance for backwards compatibility
  • Pass Curve instance as context to all functions that need it
  • Get rid of all ts-ignore in the code

0xtutti
0xtutti previously approved these changes May 8, 2025
@DanielSchiavini
Copy link
Contributor Author

Old instance is used internally 😞

+get rid of all ts-ignores
@DanielSchiavini DanielSchiavini requested a review from Copilot May 9, 2025 14:20
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors a large portion of the codebase by replacing direct references to the imported “curve” object with proper context binding (using “this” and .call(this, …)) for improved modularity and unit testing. Key changes include updates across mixins, factory functions, DAO methods, and utility functions to consistently reference instance properties and methods rather than a global “curve” import, alongside minor adjustments to type imports and documentation examples.

Reviewed Changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.

File Description
src/pools/mixins/*.ts Updated mixin methods to use “this” context and .call() for various allowance and swap calls.
src/factory/*.ts & src/dao.ts Refactored factory and DAO methods to bind “this” correctly and use updated type references.
src/constants/utils.ts Introduced a memoization helper (memoizeMethod) and updated unit conversion methods for clarity.
docs/v1/README.md Updated code examples to reflect the new context-bound method calls.
Comments suppressed due to low confidence (1)

docs/v1/README.md:659

  • [nitpick] Verify that the explicit use of .call(curve, …) in this example is intended for binding context. If the curve instance already provides the correct context, a direct method call may be simpler and clearer.
await curve.hasAllowance.call(curve, ["DAI", "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"], ['1000', '1000'], curve.signerAddress, spender)

@DanielSchiavini DanielSchiavini marked this pull request as draft May 21, 2025 13:36
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

Successfully merging this pull request may close these issues.

2 participants