search

bitshares / python-bitshares

Fully featured client-side library for the BitShares Blockchain - written entirely in python.
http://docs.pybitshares.com/
MIT License
162 stars 168 forks source link

blockchain_instance getting lost when initializing Order from id #234

Closed bitphage closed 5 years ago

bitphage commented 5 years ago

In Order: order = self.blockchain.rpc.get_objects([args[0]])[0]

self.blockchain here is a property of AbstractBlockchainInstanceProvider, and self._blockchain attr is not set, thus a new bitshares instance (shared) is being initializing effectively connecting to the default node:

> /home/vvk/devel/bitshares/python-bitshares/bitshares/price.py(109)__init__()
-> order = self.blockchain.rpc.get_objects([args[0]])[0]
(Pdb) step
--Call--
> /home/vvk/devel/bitshares/python-graphenelib/graphenecommon/instance.py(54)blockchain()
-> @property
(Pdb) next
> /home/vvk/devel/bitshares/python-graphenelib/graphenecommon/instance.py(56)blockchain()
-> if hasattr(self, "_blockchain") and self._blockchain:
(Pdb) step
> /home/vvk/devel/bitshares/python-graphenelib/graphenecommon/instance.py(60)blockchain()
-> return self.shared_blockchain_instance()

I did a quick workaround locally like this, not sure this is right fix:

--- a/bitshares/price.py
+++ b/bitshares/price.py
@@ -106,6 +106,7 @@ class Order(Price):
         if len(args) == 1 and isinstance(args[0], str):
             """ Load from id
             """
+            self._blockchain = kwargs.get('blockchain_instance')
             order = self.blockchain.rpc.get_objects([args[0]])[0]
             if order:
                 Price.__init__(self, order["sell_price"])
xeroc commented 5 years ago

Please confirm that the above commit fixes the problem for you.

bitphage commented 5 years ago

ff9701f doesn't fixes the problem as it happens inside self.blockchain.rpc.get_objects([args[0]])[0] call, not in Price.__init__()

bitphage commented 5 years ago

The problem is that Order.__init__() tries to call self.blockchain.rpc before initializing parent classes Price -> AbstractBlockchainInstanceProvider, so self._blockchain is not available.

bitphage commented 5 years ago

To reproduce:

from bitshares import BitShares
from bitshares.account import Account
from bitshares.price import Order

node = 'wss://node.testnet.bitshares.eu'
bts_testnet = BitShares(node=node)
account = Account('dtest2', blockchain_instance=bts_testnet, full=True)
orders = account.openorders

for order in orders:
    o = Order(order['id'], blockchain_instance=bts_testnet)
    print(o)

out:

deleted order 1.7.715763
deleted order 1.7.715761
deleted order 1.7.715759
deleted order 1.7.717064
deleted order 1.7.715764
deleted order 1.7.715762
deleted order 1.7.715760
bitphage commented 5 years ago

The same issue when initializing orders from dict:

orders = self.bitshares.rpc.get_limit_orders(self.market['base']['id'], self.market['quote']['id'], depth)
orders = [Order(o, bitshares_instance=self.bitshares) for o in orders]

This is working, but Order.invert() will not, because instance is lost.

xeroc commented 5 years ago

What is weird is that self.blockchain should be the instance that is passed over via blockchain_instance here and here

thehapax commented 5 years ago

Has this issue been resolved?

I've noticed in dexbot we are referencing shared_bitshares_instance() multiple times within the code, especially within the init i see several references to within multiple init() methods, including BaseStrategy, WorkerInfrastructure, etc

        self.bitshares = bitshares_instance or shared_bitshares_instance()

For an application using shared_bitshares_instance(), shouldn't it only be called once and then shared throughout the application? instead of being called and referenced multiple times?