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

API refactory #58

Closed
mpetrunic opened this issue Sep 25, 2024 · 5 comments · Fixed by #61
Closed

API refactory #58

mpetrunic opened this issue Sep 25, 2024 · 5 comments · Fixed by #61
Assignees

Comments

@mpetrunic
Copy link
Member

current api naming is not adequate and causing confusion with the users as they aren't sure which method to call. I would propose following methods:

  • bridge - calls /solution/call without contract call
  • bridgeAndCall - calls /solution/call with contract call
  • aggregateBalance - calls get_solutions_aggregation method. If there is only one network whitelisted, it will call bridge
  • aggregateBalanceAndCall - calls post_solutions_aggregation If there is only one network whitelisted, it will call bridgeAndCall
@BeroBurny BeroBurny self-assigned this Sep 26, 2024
@itsbobbyzzz168
Copy link
Collaborator

@mpetrunic @BeroBurny Liviu and I were talking through this, and suggest for the bleeding obviousnessssss

aggregateBalance -> bridgeAggregateBalance
aggregateBalanceAndCall -> bridgeAggregateBalanceAndCall

@mpetrunic
Copy link
Member Author

@mpetrunic @BeroBurny Liviu and I were talking through this, and suggest for the bleeding obviousnessssss

aggregateBalance -> bridgeAggregateBalance aggregateBalanceAndCall -> bridgeAggregateBalanceAndCall

Ooops, I've just seen this. This is a bit innacurate, since aggreateBalance doesn't neccessarilly involve bridge, it could involve, unwrapping, amm or just transfer on the same chain

@BeroBurny
Copy link
Collaborator

what about solution?

  • solution
  • solutionWithCall
  • aggregateSolutions
  • aggregateSolutionsWithCall

@mpetrunic
Copy link
Member Author

@BeroBurny that would make sense, but I think it might be confusing for the user. Maybe we need to align naming with intent more.
For example:

  • transfer - that's our single step step solution
  • transferWithHook - that's our single step solution + hook/call/xcall in the end
  • poolAssetOnDestination - that would be aggregateBalance
  • poolAsserOnDestinationWithHook - that would be aggregateBalance with Xcalls/cslls
    and in the future:
  • sweepFunds - would be new intent to collect funds from all the supported chains

@MakMuftic @itsbobbyzzz168 WDYT?

@MakMuftic
Copy link
Contributor

Yep like the last suggestion 👍 I see it is already applied 🎉

MakMuftic pushed a commit that referenced this issue Oct 24, 2024
closes #58 

BREAKING CHANGE: All functions renamed, different API and flows, please
consult docs or reach out to us directly!
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 a pull request may close this issue.

4 participants