Fix potential infinite generator in Market.trades.#150
Merged
Conversation
Since 5924ac1 was added, `Market.trades` is a generator and not a function. In it, there is a single exit condition -- `cnt >= limit`. However, this condition might never be reached for situations, where the number of returned results is less than required limit. The simplest test to verify this is to open an empty market -- `trades` will never return. Similar thing could happen if you ask for, eg, 50 trades, but the start - stop interval only has 30.
Codecov Report
@@ Coverage Diff @@
## develop #150 +/- ##
===========================================
- Coverage 43.87% 43.84% -0.03%
===========================================
Files 34 34
Lines 3305 3307 +2
Branches 718 719 +1
===========================================
Hits 1450 1450
- Misses 1637 1639 +2
Partials 218 218
Continue to review full report at Codecov.
|
Member
|
Good catch! Thanks! |
xeroc
added a commit
that referenced
this pull request
Oct 16, 2018
Release 0.2.0 Please see the content of RELEASE_NOTES.md! Changelog --------- 3ab4a35 (HEAD -> master) Merge branch 'release/0.2.0' eab5366 (release/0.2.0) bump required version of pygraphene 77feeb8 Merge remote-tracking branch 'origin/develop' into release/0.2.0 ee656d1 (origin/develop) Merge pull request #150 from jhtitor/market_trades c9845f8 Fix potential infinite generator in `Market.trades`. fa614ab hotfix #149 156f617 update of release notes 079081f Version bump 4554ea6 Release notes 0d89adb Allow to manually provide a wallet instance if needed 9f257dc Merge tag '0.2.0rc2' into develop ee4d8ac (tag: 0.2.0rc2, origin/master, origin/HEAD) Merge branch 'release/0.2.0rc2' 94448ac require proper version of pygraphene 352074d fix setup.py URL 2fd5638 Merge tag '0.2.0rc1' into develop 0c094ef (tag: 0.2.0rc1) Merge branch 'release/0.2.0rc1' 299ea58 version bump 35e5f5a simplify bitsharesbase.account 71f391b Merge pull request #147 from bitfag/fix-order-invert-repr 357aa63 Adjust "for_sale" when invert()-ing an Order 5924ac1 Allow to read more trading data #111 26a72fc Commenting #132 f1b27d1 Set default expiration to 30 seconds #132 bd69b2c Fix bitshares->blockchain attrribute #132 a19b9ed Fix #136 60eb2b9 Add custom operation f616874 Revert "fix Vesting" b430d3b Set default order expiration to 1yr a949570 fix Vesting f70f20f Ensure we don't through in case 'secs=None' 024ad38 Requirement for pygraphene>=1.0.0 0dabd9a Use PublicKey's internal sorting algorithm 1f45a03 reenable tox versions of python 8322ae9 Merge branch 'feature/storage-models' into develop d67d863 Merge branch 'develop' into feature/storage-models f974ef5 Merge pull request #141 from bitfag/fix-filled-order-history 8b7b134 Fix incompatibility with new graphene framework 9ce8068 Fix init of FilledOrder from history op 51c2277 Fix unittests for pygraphene@develop 72989a8 Merge branch 'develop' into feature/storage-models dc0f7ab Merge tag '0.1.22' into develop 803f828 further improvements 671c837 Merge branch 'develop' into feature/storage-models 305efe2 Raise Exception in case the key has been included already 5b2736a fix store to deal with proper sqlite file etc d715357 migrate more stuff to pygraphene when dealing with objects and operations 3b8070f migrate tests to pygraphene 4bf5c1b clenaup storage by moving to graphenelib 7e5ff23 unit tests now with fixtures 2cdaabd Finalize storage f5e7ede call to obtain only the store 55ab8d9 new documentation for stores 2f85c74 Finalize new API for Stores 94b2820 finalize interface 567e03b nicer interface 1e46d72 Storage, now also allows encrypted keys 79aa053 BaseKey implemnentation c7132c5 setup BaseConfig d90fe15 separate storage into subfolder
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since 5924ac1 was added,
Market.tradesis a generator and not a function. In it, there is a single exit condition --cnt >= limit. However, this condition might never be reached for situations, where the number of returned results is less than required limit.The simplest test to verify this is to open an empty market --
tradeswill never return. Similar thing could happen if you ask for, eg, 50 trades, but the start - stop interval only has 30.