bitcoin-dev-project / warnet

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

tank: use default options from constructor #159

Closed pinheadmz closed 7 months ago

pinheadmz commented 7 months ago

Fixes error logged below. If a node in a graph file doesn't have a bitcoin_config then it was being set to None instead of ""

Traceback (most recent call last):
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
    self.run()
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/threading.py", line 975, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/matthewzipkin/Desktop/work/warnet/src/warnet/server.py", line 270, in <lambda>
    t = threading.Thread(target=lambda: thread_start(wn))
                                        ^^^^^^^^^^^^^^^^
  File "/Users/matthewzipkin/Desktop/work/warnet/src/warnet/server.py", line 256, in thread_start
    wn.generate_deployment()
  File "/Users/matthewzipkin/Desktop/work/warnet/src/warnet/warnet.py", line 147, in generate_deployment
    self.container_interface.generate_deployment_file(self)
  File "/Users/matthewzipkin/Desktop/work/warnet/src/interfaces/docker_interface.py", line 232, in generate_deployment_file
    self._write_bitcoin_confs(warnet)
  File "/Users/matthewzipkin/Desktop/work/warnet/src/interfaces/docker_interface.py", line 187, in _write_bitcoin_confs
    self.write_bitcoin_conf(tank,base_bitcoin_conf)
  File "/Users/matthewzipkin/Desktop/work/warnet/src/interfaces/docker_interface.py", line 239, in write_bitcoin_conf
    options = tank.conf.split(",")
              ^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'split'
vercel[bot] commented 7 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 Oct 23, 2023 8:17pm
willcl-ark commented 7 months ago

This is (in theory) exactly why I changed these to .get() which has a default if no key exists: https://docs.python.org/3/library/stdtypes.html#dict.get

Perhaps the change should map simply be to return a new string str() instead of "" or something? We shouldn't need all those (ugly) if statements...

willcl-ark commented 7 months ago

Yes I think the only required diff should be:

diff --git a/src/warnet/tank.py b/src/warnet/tank.py
index d3ab33d..a6fcb83 100644
--- a/src/warnet/tank.py
+++ b/src/warnet/tank.py
@@ -73,7 +73,7 @@ class Tank:
                         f"Unsupported version: can't be generated from Docker images: {version}"
                     )
             self.version = version
-        self.conf = node.get("bitcoin_config")
+        self.conf = node.get("bitcoin_config", "")
         self.netem = node.get("tc_netem")
         self.extra_build_args = node.get("build_args", "")
         self.config_dir = self.warnet.config_dir / str(self.suffix)
diff --git a/test/data/v25_x_12.graphml b/test/data/v25_x_12.graphml
index d508f47..bcd75ee 100644
--- a/test/data/v25_x_12.graphml
+++ b/test/data/v25_x_12.graphml
@@ -49,7 +49,7 @@
     </node>
     <node id="11">
         <data key="version">25.0</data>
-        <data key="bitcoin_config">uacomment=w11</data>
+        <!-- no bitcoin_config to test unused options -->
     </node>
     <!-- connect the nodes in a ring to start -->
     <edge id="11" source="0" target="1"></edge>
willcl-ark commented 7 months ago

Sorry I was reading this as an issue with extra_build_args, not bitcoin_config, but I see it now...

pinheadmz commented 7 months ago

Ok I'll take your suggestion and open a different PR. I grew up in a world where default options are set in object constructor and then updated by passed-in options only if they are set by the user.