search

bitcoin-dev-project / warnet

Monitor and analyze the emergent behaviors of Bitcoin networks
https://warnet.dev
MIT License
63 stars 28 forks source link

Generate new network graphs #93

Closed willcl-ark closed 7 months ago

willcl-ark commented 8 months ago

Generate networkx.random_internet_as_graph-type graphs using CLI.

e.g.

$ warcli graph create n=100 --outfile=random_internet_as_graph_n100.graphml --random
willcl-ark commented 8 months ago

@pinheadmz if you review changed some cli commands:

warcli network start -> warcli network create <from-file [graph_file] | random>

willcl-ark commented 8 months ago

The weighted tags mean that there will likely be more nodes with recent version numbers.

We might want to add a more accurate distribution later, or allow users to pass their own distribution function into this

willcl-ark commented 8 months ago

@pinheadmz @josibake should we also allow node versionc constriants, "e.g >=0.21.1" or just leave as weighted random over all supported tags?

vercel[bot] commented 8 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
warnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 2, 2023 1:26pm
pinheadmz commented 8 months ago

should we also allow node versionc constriants, "e.g >=0.21.1" or just leave as weighted random over all supported tags?

bitcoind should be free to make these decisions. I think once we have the p2p dns seeder (which can provide service flags) this will evolve on its own

I dont love removing start, would prefer it if create is a new command that once executed either:

this latter option i think is preferable but then i also think well need a directory of graph files and a "list" method for them ?

pinheadmz commented 8 months ago

warcli network start -> warcli network create <from-file [graph_file] | random>

a little confused. you didnt remove/replace the start command ?

willcl-ark commented 8 months ago

should we also allow node versionc constriants, "e.g >=0.21.1" or just leave as weighted random over all supported tags?

bitcoind should be free to make these decisions. I think once we have the p2p dns seeder (which can provide service flags) this will evolve on its own

Whilst the p2p dns seeder will certainly help in making connections, the bitcoind container cannot decide it's own binary version number it's going to run? This PR is about generating a graph file of randomised (or not) node:versions...

My question was about whether we should have warcli network create random take a min_version param (or similar), in the case that people might not want v16.3 nodes for example

I dont love removing start, would prefer it if create is a new command that once executed either:

* also starts the network as if `start` had also been executed

* save the graph file somewhere and tell the user its name so the user can manully call `start`

this latter option i think is preferable but then i also think well need a directory of graph files and a "list" method for them ?

Should we simply remove the grammar at this point, if we are not going to use it for everything? Currently we have:

  debug      Debug commands
   ├─   generate-compose  Generate the docker-compose file for a given...
   ├─   update-dns-seed   Update the dns seed database using a <graph_file> on...
  debug-log  Fetch the Bitcoin Core debug log from <node> in [network]
  help       Display help information for the given command.
  messages   Fetch messages sent between <node_a> and <node_b> in <network>
  network    Network commands
   ├─   down   Run 'docker-compose down on a warnet named <--network> (default:...
   ├─   start  Start a warnet with topology loaded from a <graph_file> into...
   ├─   up     Run 'docker-compose up' on a warnet named <--network> (default:...
  rpc        Call bitcoin-cli <method> <params> on <node> in <--network>
  scenarios  Scenario commands
   ├─   active  List running scenarios on <--network> (default=warnet) as...
   ├─   list    List available scenarios in the Warnet Test Framework
   ├─   run     Run <scenario> from the Warnet Test Framework on <--network>...
   ├─   stop    Stop scenario with <pid> from running on <--network>
  stop       Stop warnetd.

@josibake felt like network was un-intuative here and would prefer start and stop as top-level. I'd argue that create belongs in network, and you now think that start and stop belong in network but not create :P

I am very happy to go with the majority, but I think we should spec out exactly how we want this to operate so that we can build it just so!

My personal perference would be to retain the sub-commands (but use them in all cases), but happy to be out-voted on this one...

edit: perhaps on a re-read now, I see that you would be for having create (as a network sub-command?) but just have it generate the graph file? I could live with that!)

pinheadmz commented 8 months ago

ok yeah we got wires crossed.

josibake commented 8 months ago

Just poppin in to say ignore what I was saying earlier, been playing around with the CI more and got a feel for it. I do think more detailed help messages could alleviate any "un-intuitiveness"

willcl-ark commented 8 months ago

OK how about this push? New sub-command graph, which will do graph things. Like generate a random graph!

try

warcli graph generate 20 0.3 to see one spat out to the terminal. or add --file=/path/to/file to have it dumped to a file?

willcl-ark commented 8 months ago 
* Create a network with random bitcoind versions: great, got it now. yeah weighting is a cool idea here based on current user agent distribution. This is essentially an automatic warnet generation function and mine as well have this feature. Maybe it can be behind a switch where default is weighted random versions but `--allversions=25.0` or something is a flag

Yeah I think we could add an --allversions flag very easily!

* grammar:
  I DO LIKE:
  `warcli network create random1`
  `warlci network start random1`
  `warlci network stop random1`
  I think we could also add, example:

See latest force-push, again, happy to be overruled on this, but perhaps graph sub-command makes even more sense (but takes more steps to fire up?)

willcl-ark commented 8 months ago

Force-pushed again, ready for review. Updated README, PR description and commits 👍🏼

pinheadmz commented 8 months ago

help?

--> warcli graph create triad_graph
Parsed params: {}
Failed to create graph: triad_graph() missing 1 required positional argument: 'triad_name'
--> warcli graph create triad_graph name1
Invalid parameter format: name1
--> warcli graph create triad_graph --triad_name=name1
Usage: warcli graph create [OPTIONS] GRAPH_TYPE [PARAMS]...
Try 'warcli graph create --help' for help.

Error: No such option: --triad_name
pinheadmz commented 8 months ago

help? pt. 2

--> warcli graph create bull_graph
Parsed params: {}
Traceback (most recent call last):
  File "/opt/homebrew/bin/warcli", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/matthewzipkin/Desktop/work/warnet/src/warnet/cli/graph.py", line 173, in create
    pos = nx.spring_layout(graph)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/networkx/utils/decorators.py", line 766, in func
    return argmap._lazy_compile(__wrapper)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<class 'networkx.utils.decorators.argmap'> compilation 8", line 3, in argmap_spring_layout_5
  File "/opt/homebrew/lib/python3.11/site-packages/networkx/utils/misc.py", line 257, in create_random_state
    import numpy as np
ModuleNotFoundError: No module named 'numpy'
josibake commented 8 months ago 
import numpy as np

ModuleNotFoundError: No module named 'numpy'

huh, this should have been handled as a dependency when install networkx. @willcl-ark might need to refresh the package dependencies, or @pinheadmz maybe you just need to re-run pip install for the branch?

willcl-ark commented 7 months ago

Right, I had commits from https://github.com/bitcoin-dev-project/warnet/pull/141 in here so that the cleanup is not so error-filled. Looks like removing them causes CI to fail, so I think we should look at that first @pinheadmz

pinheadmz commented 7 months ago

also, now that we got tests cleaning up properly that don't start networks, I think we should start networks in this test ;-) just to ensure that the generated graph files work.

willcl-ark commented 7 months ago

also, now that we got tests cleaning up properly that don't start networks, I think we should start networks in this test ;-) just to ensure that the generated graph files work.

Also did this now. Make sense to have a total end-to-end test.

While I was toying with how much work it would be to switch to pytest I did notice too that we have many tests which re-use the same TestBase object and so could be influenced by a previous test...

pinheadmz commented 7 months ago

got this error running the test:

Waiting for all tanks to reach 'running': {'total': 5, 'running': 5}
generated graph created network test passed
killing server with pid 20812

Remaining server output:
2023-10-13 10:01:15,342 - warnet - DEBUG - {'jsonrpc': '2.0', 'method': 'status', 'params': {'network': 'warnet_test_62_k9a_y'}, 'id': 6}
2023-10-13 10:01:15,409 - werkzeug - INFO - 127.0.0.1 - - [13/Oct/2023 10:01:15] "POST /api HTTP/1.1" 200 -

killing server with pid 20812
Exception ignored in atexit callback: <bound method TestBase.cleanup of <test_base.TestBase object at 0x103457550>>
Traceback (most recent call last):
  File "/Users/matthewzipkin/Desktop/work/warnet/test/test_base.py", line 70, in cleanup
    self._cleanup_server()
  File "/Users/matthewzipkin/Desktop/work/warnet/test/test_base.py", line 63, in _cleanup_server
    os.killpg(os.getpgid(self.server.pid), signal.SIGTERM)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
ProcessLookupError: [Errno 3] No such process
willcl-ark commented 7 months ago

got this error running the test:

Waiting for all tanks to reach 'running': {'total': 5, 'running': 5}
generated graph created network test passed
killing server with pid 20812

Remaining server output:
2023-10-13 10:01:15,342 - warnet - DEBUG - {'jsonrpc': '2.0', 'method': 'status', 'params': {'network': 'warnet_test_62_k9a_y'}, 'id': 6}
2023-10-13 10:01:15,409 - werkzeug - INFO - 127.0.0.1 - - [13/Oct/2023 10:01:15] "POST /api HTTP/1.1" 200 -

killing server with pid 20812
Exception ignored in atexit callback: <bound method TestBase.cleanup of <test_base.TestBase object at 0x103457550>>
Traceback (most recent call last):
  File "/Users/matthewzipkin/Desktop/work/warnet/test/test_base.py", line 70, in cleanup
    self._cleanup_server()
  File "/Users/matthewzipkin/Desktop/work/warnet/test/test_base.py", line 63, in _cleanup_server
    os.killpg(os.getpgid(self.server.pid), signal.SIGTERM)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
ProcessLookupError: [Errno 3] No such process

That's odd. We should handle that in any case, but I don't see the same?

pinheadmz commented 7 months ago

also saw this at the end of the test. I feel like if the test is succesful it should stop the server using warcli stop and not have to kill PID. although hm. maybe its trying to read log file after its been deleted?

Stopping network
Error stopping server: 'NoneType' object has no attribute 'read'
Attempting to cleanup docker network
killing server with pid 207604
willcl-ark commented 7 months ago

also saw this at the end of the test. I feel like if the test is succesful it should stop the server using warcli stop and not have to kill PID. although hm. maybe its trying to read log file after its been deleted?


Stopping network

Error stopping server: 'NoneType' object has no attribute 'read'

Attempting to cleanup docker network

killing server with pid 207604

Agree, I'll address later this eve

pinheadmz commented 7 months ago

tried to create a sudoku_graph but failed:

ModuleNotFoundError: No module named 'scipy'

do some of these graphs require certain dependencies? yeesh

pinheadmz commented 7 months ago

Related to this PR: we have example graph files in three places now: src/templates test/data graphs/

The last one is where Martin went to find a graph file to play with. I propose we remove example.graphml entirely and add a default.graphml to graphs/ that is easy (i.e. 12 v25.0 nodes with no branch building) so it can be started quickly by new users

willcl-ark commented 7 months ago

@pinheadmz updated this for you

pinheadmz commented 7 months ago

need to add lxml to requirements

willcl-ark commented 7 months ago

need to add lxml to requirements

₿ pip uninstall warnet; and pip freeze | xargs pip uninstall; and pip install -e .
...
₿ warcli graph create n=100 --outfile=random_internet_as_graph_n100.graphml --random
Generated graph written to file: random_internet_as_graph_n100.graphml

Works for me without that (tm) 🤷🏼‍♂️

willcl-ark commented 7 months ago

OK ready FR FR

pinheadmz commented 7 months ago

hm yeah i didnt get the lxml error this time... ooooohhhkkkaaayyyyyy

pinheadmz commented 7 months ago

YES.