-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Soroban] Add token parsing/formatting functions #106
[Soroban] Add token parsing/formatting functions #106
Conversation
@jeesunikim that's a very good point. @piyalbasu @aristidesstaffieri would you guys know the reason why we've created our own version of those functions in Freighter? Is this because of some type compatibility issue? Thanks! |
ah @CassioMG, my bad — I had this task [dApps] Move common formatters from Freighter/dApps to shared libraries that I never got to complete because I was waiting for some PRs to be merged :hide_homer:. I was told not to work on importing formatters for dApps since dApps required so dependencies updates and they weren't prioritized. I can work on this for Freighter tomorrow. |
I think it's just that they existed in Freighter first and we haven't gotten around to using the ones in the sdk yet. |
|
@Ifropc do you mean creating a separate module like
I like this idea
What do you mean by this exactly? I know it is advised to not import
|
Sorry for the confusion here - I didn't realize If @Ifropc 's suggestion is to create a In reference to Another thought: is there any benefit to re-exporting all of stellar-sdk in this library? In this case, as a dev, I could just install |
Oh my point is that if it's something that is already in base SDK we should just use that functionality instead (i.e. not reinventing the wheel) -> not related to this PR, as frankly I'm not familiar with Soroban codebase at all, but just in general thought for the future. |
+1 for a @CassioMG if you point the PR to release/1.4.0 it has the change of using a monorepo. And then the setup can be the same as |
Hey guys, thanks for all the feedback and sorry about the late response - I was OOO last week I'm planning on pointing my [Soroban] PRs to As in regards to the token parsing/formatting functions I think I'll just re-export from stellar-sdk as you guys suggested. Let me know in case you have further comments - thanks! |
Thanks for the ping! For the specific case of the formatting and parsing functions, those were a feature added by @jeesunikim that felt useful and fundamental enough to merge into I've discussed this a lot with folks like @piyalbasu and @chadoh in the context of the TypeScript bindings in the Soroban CLI as well as the related generated However, I was wary of this because of a similar fear as @Ifropc:
I think it would be very valuable to have a plug-and-play layer above the SDK with high-level, Soroban-focused functionality. I don't have strong opinions on its naming or functionality, but to me it does seem very clear that there is a layer between the stellar-sdk (which, traditionally, has been XDR and HTTP wrappers) and wallets (which do a LOT) that is missing for Soroban. Formatters, generated clients, etc. (and, arguably, even things like
Would there be value in just a |
Just as a heads-up, even though we named the SDK "wallet-sdk" even now it extends in usage above just the wallet integrations. We still in a "figuring out" stage of what the future ask is, but even now the wallet SDK can (and should) be used on the backend too. |
Hey everyone, I'm closing this PR in favor of #120 which adds the token parsing/formatting functions to the new soroban workspace. As you can check there it is now re-exporting the existing functions from stellar-base. Please feel free to add any comments there as well, thanks! |
https://stellarorg.atlassian.net/jira/software/c/projects/WAL/boards/37?selectedIssue=WAL-1367
Differences when compared to Freighter's implementation:
bigint | BigNumber | number | string
;parseTokenAmount
function returnsbigint
instead ofBigNumber
sincebigint
seems to be the default numbering format used in the soroban env;