Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[spec] Leading + character when parsing amount #101

Open
antonok-edm opened this issue Mar 11, 2022 · 4 comments
Open

[spec] Leading + character when parsing amount #101

antonok-edm opened this issue Mar 11, 2022 · 4 comments

Comments

@antonok-edm
Copy link

@chitalian and I are working on a Rust port of the @solana/pay JavaScript SDK. Many float implementations, including the one in the Rust standard library and the BigNumber library used by the official SDK, consider a leading + to be valid when parsing numbers. We noticed that the description in the spec does not address the behavior of a leading + character. For reference, it looks like the official SDK happens to forbid a leading +, although it's unclear from the code whether or not it's intentional. Forbidding a leading + feels like the correct choice, but it would be great to have that clarified in the spec.

@antonok-edm antonok-edm changed the title Leading + character when parsing amount [spec] Leading + character when parsing amount Mar 11, 2022
@jordaaash
Copy link
Collaborator

Yes, forbidding a leading plus would be good. This was intentional in the reference implementation, but we could stand to make the spec more strict.

Also, awesome project! I can't view your Gitlab, so I'm guessing it's still private?

@jordaaash
Copy link
Collaborator

Since you're doing an awesome job of reviewing the spec and finding places where it's ambiguous or inconsistent, would you and @chitalian take a look at #77 and the latest spec there?

Transfer requests should be mostly the same, but transaction requests are new. Wallets are actively implementing this now, but it's still open for feedback!

@antonok-edm
Copy link
Author

antonok-edm commented Mar 14, 2022

Yes, forbidding a leading plus would be good. This was intentional in the reference implementation, but we could stand to make the spec more strict.

Sounds good 😄

Also, awesome project! I can't view your Gitlab, so I'm guessing it's still private?

Yep, we're in the never-ending "almost ready to publish" phase 😅

We'll be submitting it as an entry in the Riptide hackathon though, so it'll be up soon! You're welcome to add it to the Coming soon section Supporting Wallets in the meantime - we're using it for transfer requests.

Edit: it's published now!

would you and @chitalian take a look at #77 and the latest spec there?

I took a quick scan over it, I will add my thoughts there.

@antonok-edm
Copy link
Author

Also, awesome project! I can't view your Gitlab, so I'm guessing it's still private?

Just realized our solana-pay repo was also private, which we didn't intend 😅

That's public now here, and also on crates.io.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants