[alpha2] move from twisted to asyncio + new network #934
Conversation
…ommits) Squashed commits: [8a1cdf0e] - resolve memory leak in VM - resolve notification exception [86ff17cf] - fix gas limit check - some application engine updates according to NeoVM 2.4.1 [8627a8a5] - prevent printing if a node is already removed - fix access + exception - gracefully handle port in use exception - change startup sequence of syncmgr/nodemgr/prompt - handle prompt_toolkit cleanup failure on exit - fix edge case in replace_node using wrong loop during disconnect - add temporary block health check - make sure replace_node tags addresses as bad rebase [fa0cfb9] process feedback [7469e3b] enable request logging [98f0a3d] process feedback [b29bbad] missed file :| [afa7af6] resolve rebase test regression [97ed78d] try travis 3.7 fix [3657513] update travis for python 3.7 [90b42a3] update requirements, set node to use 2.10.1 code, fix typo [cb43f69] - make sure new wallet gets synced when created - cleanup logger stuff - ensure we can't persist blocks that are out of order - remove tcp session hack [6028e52] cleanup and fix issues after rebase [57930ef] New network code + move from twisted to asyncio - add hack to work around max 1 request per hash session limit causing out of sync - more logic to shutdown cleanly (+9 squashed commits) Squashed commits: [59c5916] - move mempool out of nodemanager - add relay cache + logic - add getdata support in node for relay cache items only [1d081e9] add helper logic to avoid trying to persist blocks multiple times and keeping wallet height in check [c20cd6e] fix node.relay() , remove property for size in message [383ad47] - Add new ping payload support - resolve send_addr issue - cleanup unused functions, add missing typing [8bed164] clean up prints to use logger, remove obsolete code [9791737] Update db schema, ensure setting up an asyncio loop before initialising a DB, don't manually stop loop after run complete [1499f91] - update requirements - fix /examples/ - fix `set maxpeers` not adding new CLI clients - fix exceeding max nodes connection count - add IPfiltering (blacklist/whitelist) - fix show nodes extra flags not working with 0 connected nodes - add code for more graceful shutdown preventing stack traces and db corruption - update trouble shooting section after OSX update when `scrypt` module fails. [2743930] cleanup [ee2f4c6] New network code + move from twisted to asyncio cleanup - update requirements - fix /examples/ - fix `set maxpeers` not adding new CLI clients - fix exceeding max nodes connection count - add IPfiltering (blacklist/whitelist) - fix show nodes extra flags not working with 0 connected nodes - add code for more graceful shutdown preventing stack traces and db corruption - update trouble shooting section after OSX update when `scrypt` module fails. Update db schema, ensure setting up an asyncio loop before initialising a DB, don't manually stop loop after run complete clean up prints to use logger, remove obsolete code - Add new ping payload support - resolve send_addr issue - cleanup unused functions, add missing typing fix node.relay() , remove property for size in message add helper logic to avoid trying to persist blocks multiple times and keeping wallet height in check - move mempool out of nodemanager - add relay cache + logic - add getdata support in node for relay cache items only - add hack to work around max 1 request per hash session limit causing out of sync - more logic to shutdown cleanly
|
TODO's
|
- currently "Block-cache length" only displays zero - this fix displays the actual value
- adds parameter descriptions for new params - adds testing for new params - separately, fixes a test to avoid a RuntimeWarning
- instead of doing nothing, KeyboardInterrupt now mutes stdout while the user inputs a command - this is especially useful while `config output` includes any DEBUG setting
fix `show state` [2]
improve user input for prompt with DEBUG output [2]
improve `show nodes` [2]
|
|
||
| async def start(self): | ||
| host = 'localhost' | ||
| port = 8888 # settings.NODE_PORT |
There was a problem hiding this comment.
why is settings.NODE_PORT replaced here?
There was a problem hiding this comment.
Just a local test thing that slipped in. The default port conflicts with my privnet
There was a problem hiding this comment.
- reminder to revert to
settings.NODE_PORTprior to merging
- adds `config minpeers` and tests - prompt now mirrors api_server logic for minpeers & maxpeers
|
@ixje I am not sure what changed but I got the same error from #930 trying to access |
- previously only "pretty printed" if "verbose", didn't actually complete the command - fixed the tests to verify expected behavior - separately, now closes the wallet when exiting a prompt session
…nto asyncio * 'asyncio_alpha2' of https://github.com/ixje/neo-python: improve user input for prompt with DEBUG output improve `show nodes` fix `show state`
|
I ran into that as well. I have that fixed for |
- add tests for syncmanager
- fix a node disconnect issue - change Persist to no longer block the whole process when processing a block with many transactions by yielding control - limit max peers to 10
|
So I found an issue where a |
Absolutely! I really like the fix. Definitely an elegant solution! I wish I had thought of that but I hadn't gotten that far yet so I didn't think to reference it, lol. |
|
So the good part is; I was able to sync test from scratch to nearly max height and still be connected to nodes. The 'bad' is that I found that the With some manual wizardry (=without starting) I unlocked it and I can say it's still running stable and still connected to nodes after nearly 30 hours 🎉 . I need to find a way to reproduce the behaviour in a test and then resolve it. That should be it for getting it merged. I need to get this thing in and do smaller PRs because it's getting really hard to port back and forth between dev and this. |
|
Using the latest commit there are still similar issues shutting down as the ones I mentioned above. Again, it appears that there are still instances of |
|
I am noticing an odd behavior whenever relaying a transaction: Regardless of the Notice |
|
Using pretty print makes If this is expected and desired, I recommend removing the @ixje if you let me know how you'd like to proceed, I think I can submit a PR to resolve this issue. |
Well, that is not expected, but the root cause is simple. neo-python/neo/Core/TX/Transaction.py Line 10 in fe90f62 it looks like we missed removing a logzero usage entry |
I think we should extend the pretty print to add aliasing output |
Update build_run test
add minpeers features
fixes `wallet verbose`
|
@ixje, I'm sure you're aware but I just wanted to put in a reminder to
|
|
Also, I wanted to bring this up again: I still see a deprecation warning during Example: Notice |
@ixje I found one more instance in Send.py. I think that is all that needs fixing and addressed the issue in this PR: https://github.com/ixje/neo-python/pull/17 |
I addressed this issue in https://github.com/ixje/neo-python/pull/18 |
…nto asyncio * 'asyncio_alpha2' of https://github.com/ixje/neo-python: update build_run test for --i input process feedback fixes `wallet verbose` - previously only "pretty printed" if "verbose", didn't actually complete the command - fixed the tests to verify expected behavior add minpeers features
|
|
||
| async def start(self): | ||
| host = 'localhost' | ||
| port = 8888 # settings.NODE_PORT |
There was a problem hiding this comment.
@ixje it looks like this was not reverted prior to merging
3 participants
Continuation of #930
This PR ripped the 3rd party event-driven framework
twistedout ofneo-pythonand replaced it with the standard libraryasyncio. It also introduces new network code.neo-pythondoesn't have 100% test coverage and this PR touches pretty much every component, therefore I'm looking for help with general testing of this branch. This can be anything from running the RPC server, to general interactions like opening/closing/rebuilding wallets, sending assets, searching contracts. Whatever feature is in there should have no regression and function as expected. If it shows unexpected behaviour (read the note below first) then please drop a comment here, also for any errors you think shouldn't be there/printed .Important notes
np-bootstrapas suggested will not work as the bootstrap files have not yet been updated.neo-cli->v2.10.1. Therefore when using this in a private-net environment make sure you run it against nodes withneo-cliv2.10.1. Upgradeneo-privatenetusing:neo-localis not yet updated afaik.To elaborate why this is important: the
>= 2.9.x <= 2.10.0series ofneo-clistarted tracking data sent to a peer and limit requesting the same data to only once per session. In order to keep up to date with the best height of a connected node in those series we have 2 optionsI found neither approach acceptable and convinced NEO to add a new network command that allows requesting the best height of a peer without disconnecting. Without going into too much detail, the new network code uses this information to avoid requesting data from a node that doesn't have the data in the first place. The impact for now is that neo-cli nodes <
2.10.1basically cannot stay in sync. It will be a transition period that we'll have to live with.