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

fix: safe intDecoding #599

Merged
merged 3 commits into from
Jun 29, 2022
Merged

fix: safe intDecoding #599

merged 3 commits into from
Jun 29, 2022

Conversation

AlgoDoggo
Copy link
Contributor

This PR fixes a couple of bugs with safe mode for integer decoding during json parse:

  • safe mode threw errors if 10**15 > value < Number.MAX_SAFE_INTEGER . It would have shown in the unit test if we had tested for edge cases.
  • safe mode did not decode integers as numbers. It didn't alter the output of JSONbig in any way.
    It is again only an issue for those values that are 10**15 > value < Number.MAX_SAFE_INTEGER

Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Thank you for finding and fixing this bug

@jasonpaulos jasonpaulos merged commit 69a7394 into algorand:develop Jun 29, 2022
@AlgoDoggo AlgoDoggo deleted the fix-json branch June 30, 2022 08:08
algochoi added a commit that referenced this pull request Aug 8, 2022
* properly set trace maxWidth (#593)

* fix: safe intDecoding (#599)

* fix: safe intDecoding

* test: parse number in edge case

* refactor: remove optional chaining for node12

* Remove code that relies on node's path module (#598)

* Remove code that relies on node's path module

* Replace url-parse with built in WHATWG URL API

* Removed path-browserify fallback from webpack config

* Removed path-browserify and url-parse from npm dependencies

* Removed references to `path-browserify` in FAQ.md

* bump version

* Github-Actions: Adding PR title and label checks (#600)

* Remove unused/unmaintained templates (#607)

* Dev Tools: Source map decoder (#590)

* adding source map decoder

* Enhancement: Upgrade typedoc and plugins (#605)

* Update ts-node, typescript, and typedoc to latest

* docs: tealSign (#610)

* bump version and add to changelog

* update README.md for new version

* API: Support attaching signatures to standard and multisig transactions (#595)

* Add attach signature method to transaction class

* Add multisig external signature methods

* Fix failing multisig test

* Add signature length checks

* Add method to create an unsigned multisig transaction blob

* Rename multisig create methods and use unencoded transaction

* Refactor `createMultisigTransactionWithSignature` to use `createMultisigTransaction` method

* Fix algosdk createMultisigTransaction export

* Use MultisigMetadata without pks in new create method

* These types should be consolidated in the future, and addrs seems like a better convention to use long-term

* More descriptive test suite name

* AVM:  Consolidate TEAL and AVM versions (#609)

* Testing: Use Dev mode network for cucumber tests (#614)

* Send zero txn to itself

* Refactor block advance functions

* Revise steps to allow for rekeying transient accounts

* Try to reduce flaky tests

* Move constant into step

* Add artificial sleep instead of sending blank txns when mimicking wait for block API

* Reduce flaky tests

* Remove unnecessary use of v2 algod client (#616)

* Remove unnecessary use of v2 algod client
* Add missing await for async function calls (#617)

* Rename rekey tag in makefile

* Revert testing branch back to master

Co-authored-by: Michael Diamant <[email protected]>

* Revert package lock from develop

Co-authored-by: Joe Polny <[email protected]>
Co-authored-by: AlgoDoggo <[email protected]>
Co-authored-by: Bryan Dela Cruz <[email protected]>
Co-authored-by: Lucky Baar <[email protected]>
Co-authored-by: Jack <[email protected]>
Co-authored-by: Eric Warehime <[email protected]>
Co-authored-by: Ben Guidarelli <[email protected]>
Co-authored-by: Fionna Chan <[email protected]>
Co-authored-by: Jack Smith <[email protected]>
Co-authored-by: Jacob Daitzman <[email protected]>
Co-authored-by: Michael Diamant <[email protected]>
ahangsu added a commit that referenced this pull request Sep 6, 2022
* Add boxes endpoint with path tests

* Add lookup box by name api for indexer

* Merge develop branch into feature/box-storage (#620)

* properly set trace maxWidth (#593)

* fix: safe intDecoding (#599)

* fix: safe intDecoding

* test: parse number in edge case

* refactor: remove optional chaining for node12

* Remove code that relies on node's path module (#598)

* Remove code that relies on node's path module

* Replace url-parse with built in WHATWG URL API

* Removed path-browserify fallback from webpack config

* Removed path-browserify and url-parse from npm dependencies

* Removed references to `path-browserify` in FAQ.md

* bump version

* Github-Actions: Adding PR title and label checks (#600)

* Remove unused/unmaintained templates (#607)

* Dev Tools: Source map decoder (#590)

* adding source map decoder

* Enhancement: Upgrade typedoc and plugins (#605)

* Update ts-node, typescript, and typedoc to latest

* docs: tealSign (#610)

* bump version and add to changelog

* update README.md for new version

* API: Support attaching signatures to standard and multisig transactions (#595)

* Add attach signature method to transaction class

* Add multisig external signature methods

* Fix failing multisig test

* Add signature length checks

* Add method to create an unsigned multisig transaction blob

* Rename multisig create methods and use unencoded transaction

* Refactor `createMultisigTransactionWithSignature` to use `createMultisigTransaction` method

* Fix algosdk createMultisigTransaction export

* Use MultisigMetadata without pks in new create method

* These types should be consolidated in the future, and addrs seems like a better convention to use long-term

* More descriptive test suite name

* AVM:  Consolidate TEAL and AVM versions (#609)

* Testing: Use Dev mode network for cucumber tests (#614)

* Send zero txn to itself

* Refactor block advance functions

* Revise steps to allow for rekeying transient accounts

* Try to reduce flaky tests

* Move constant into step

* Add artificial sleep instead of sending blank txns when mimicking wait for block API

* Reduce flaky tests

* Remove unnecessary use of v2 algod client (#616)

* Remove unnecessary use of v2 algod client
* Add missing await for async function calls (#617)

* Rename rekey tag in makefile

* Revert testing branch back to master

Co-authored-by: Michael Diamant <[email protected]>

* Revert package lock from develop

Co-authored-by: Joe Polny <[email protected]>
Co-authored-by: AlgoDoggo <[email protected]>
Co-authored-by: Bryan Dela Cruz <[email protected]>
Co-authored-by: Lucky Baar <[email protected]>
Co-authored-by: Jack <[email protected]>
Co-authored-by: Eric Warehime <[email protected]>
Co-authored-by: Ben Guidarelli <[email protected]>
Co-authored-by: Fionna Chan <[email protected]>
Co-authored-by: Jack Smith <[email protected]>
Co-authored-by: Jacob Daitzman <[email protected]>
Co-authored-by: Michael Diamant <[email protected]>

* Revert package lock

* Fix comments in box related APIs

* Add next token to indexer box API test

* rm unused step

* Update src/client/v2/algod/getApplicationBoxByName.ts

* Update .test-env

Co-authored-by: Joe Polny <[email protected]>
Co-authored-by: AlgoDoggo <[email protected]>
Co-authored-by: Bryan Dela Cruz <[email protected]>
Co-authored-by: Lucky Baar <[email protected]>
Co-authored-by: Jack <[email protected]>
Co-authored-by: Eric Warehime <[email protected]>
Co-authored-by: Ben Guidarelli <[email protected]>
Co-authored-by: Fionna Chan <[email protected]>
Co-authored-by: Jack Smith <[email protected]>
Co-authored-by: Jacob Daitzman <[email protected]>
Co-authored-by: Michael Diamant <[email protected]>
Co-authored-by: Hang Su <[email protected]>
Co-authored-by: Hang Su <[email protected]>
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.

3 participants