Skip to content

Commit

Permalink
Update HTTP status codes returned by RPC-API
Browse files Browse the repository at this point in the history
Some of the return codes for the endpoints in
the RPC API were incorrect, these are fixed.
They are also checked in the tests.
Additionally, an extra test of the maker start/
stop function is added (though it tests only
state updates, not actual service start).
  • Loading branch information
AdamISZ committed Dec 27, 2021
1 parent 76cb730 commit e6ea9d0
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 17 deletions.
7 changes: 4 additions & 3 deletions jmclient/jmclient/wallet_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ def dummy_restart_callback(msg):
# respond to requests, we return the status to the client:
if('seedphrase' in kwargs):
return make_jmwalletd_response(request,
status=201,
walletname=self.wallet_name,
token=encoded_token,
seedphrase=kwargs.get('seedphrase'))
Expand Down Expand Up @@ -556,7 +557,7 @@ def setup():
self.services["maker"].startService()
except YieldGeneratorServiceSetupFailed:
raise ServiceAlreadyStarted()
return make_jmwalletd_response(request)
return make_jmwalletd_response(request, status=202)

@app.route('/wallet/<string:walletname>/maker/stop', methods=['GET'])
def stop_maker(self, request, walletname):
Expand All @@ -569,7 +570,7 @@ def stop_maker(self, request, walletname):
CJ_MAKER_RUNNING:
raise ServiceNotStarted()
self.services["maker"].stopService()
return make_jmwalletd_response(request)
return make_jmwalletd_response(request, status=202)

@app.route('/wallet/<string:walletname>/lock', methods=['GET'])
def lockwallet(self, request, walletname):
Expand Down Expand Up @@ -834,4 +835,4 @@ def dummy_user_callback(rel, abs):

_, self.coinjoin_connection = start_reactor(dhost, dport,
self.clientfactory, rs=False)
return make_jmwalletd_response(request)
return make_jmwalletd_response(request, status=202)
77 changes: 63 additions & 14 deletions jmclient/test/test_wallet_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from jmbitcoin import CTransaction
from jmclient import (load_test_config, jm_single, SegwitWalletFidelityBonds,
JMWalletDaemon, validate_address, start_reactor)
from jmclient.wallet_rpc import api_version_string
from jmclient.wallet_rpc import api_version_string, CJ_MAKER_RUNNING, CJ_NOT_RUNNING
from commontest import make_wallets
from test_coinjoin import make_wallets_to_list, sync_wallets

Expand Down Expand Up @@ -142,10 +142,8 @@ def do_request(self, agent, method, addr, body, handler, token=None):
@defer.inlineCallbacks
def response_handler(self, response, handler):
body = yield readBody(response)
# these responses should always be 200 OK.
assert response.code == 200
# handlers check the body is as expected; no return.
yield handler(body)
yield handler(body, response.code)
return True

@defer.inlineCallbacks
Expand Down Expand Up @@ -198,14 +196,16 @@ def test_create_list_lock_unlock(self):
self.process_unlock_response)


def process_create_wallet_response(self, response):
def process_create_wallet_response(self, response, code):
assert code == 201
json_body = json.loads(response.decode("utf-8"))
assert json_body["walletname"] == testfileloc
self.jwt_token = json_body["token"]
# we don't use this in test, but it must exist:
assert json_body["seedphrase"]

def process_list_wallets_response(self, body):
def process_list_wallets_response(self, body, code):
assert code == 200
json_body = json.loads(body.decode("utf-8"))
assert json_body["wallets"] == [testfileloc]

Expand Down Expand Up @@ -237,13 +237,15 @@ def test_direct_send_and_display_wallet(self):
yield self.do_request(agent, b"GET", addr, None,
self.process_wallet_display_response)

def process_direct_send_response(self, response):
def process_direct_send_response(self, response, code):
assert code == 200
json_body = json.loads(response.decode("utf-8"))
assert "txinfo" in json_body
# TODO tx check
print(json_body["txinfo"])

def process_wallet_display_response(self, response):
def process_wallet_display_response(self, response, code):
assert code == 200
json_body = json.loads(response.decode("utf-8"))
latest_balance = float(json_body["walletinfo"]["total_balance"])
jlog.info("Wallet display currently shows balance: {}".format(
Expand All @@ -266,6 +268,47 @@ def test_getaddress(self):
yield self.do_request(agent, b"GET", addr, None,
self.process_new_addr_response)

@defer.inlineCallbacks
def test_maker_start_stop(self):
""" Tests that we can start the maker service.
As for the taker coinjoin test, this is currently
a simple/artificial test, only checking return status
codes and state updates, but not checking that an actual
backend maker service is started.
"""
self.daemon.auth_disabled = True
agent = get_nontor_agent()
addr_start = self.get_route_root()
addr_start += "/wallet/"
addr_start += self.daemon.wallet_name
addr = addr_start + "/maker/start"
addr = addr.encode()
body = BytesProducer(json.dumps({"txfee": "0",
"cjfee_a": "1000", "cjfee_r": "0.0002",
"ordertype": "reloffer", "minsize": "1000000"}).encode())
yield self.do_request(agent, b"POST", addr, body,
self.process_maker_start)
# For the second phase, since we are not currently processing
# via actual backend connections, we need to mock the client
# protocol instance that requests shutdown of all message channels:
class DummyMakerClientProto(object):
def request_mc_shutdown(self):
jlog.info("Message channel shutdown request registered.")
self.daemon.services["maker"].clientfactory.proto_client = \
DummyMakerClientProto()
addr = addr_start + "/maker/stop"
addr = addr.encode()
yield self.do_request(agent, b"GET", addr, None,
self.process_maker_stop)

def process_maker_start(self, request, code):
assert code == 202
assert self.daemon.coinjoin_state == CJ_MAKER_RUNNING

def process_maker_stop(self, request, code):
assert code == 202
assert self.daemon.coinjoin_state == CJ_NOT_RUNNING

@defer.inlineCallbacks
def test_gettimelockaddress(self):
self.daemon.auth_disabled = True
Expand All @@ -278,7 +321,8 @@ def test_gettimelockaddress(self):
yield self.do_request(agent, b"GET", addr, None,
self.process_new_addr_response)

def process_new_addr_response(self, response):
def process_new_addr_response(self, response, code):
assert code == 200
json_body = json.loads(response.decode("utf-8"))
assert validate_address(json_body["address"])[0]

Expand All @@ -294,7 +338,8 @@ def test_listutxos(self):
yield self.do_request(agent, b"GET", addr, None,
self.process_listutxos_response)

def process_listutxos_response(self, response):
def process_listutxos_response(self, response, code):
assert code == 200
json_body = json.loads(response.decode("utf-8"))
# some fragility in test structure here: what utxos we
# have depend on what other tests occurred.
Expand All @@ -315,17 +360,20 @@ def test_session(self):
yield self.do_request(agent, b"GET", addr, None,
self.process_session_response)

def process_session_response(self, response):
def process_session_response(self, response, code):
assert code == 200
json_body = json.loads(response.decode("utf-8"))
assert json_body["maker_running"] is False
assert json_body["coinjoin_in_process"] is False

def process_unlock_response(self, response):
def process_unlock_response(self, response, code):
assert code == 200
json_body = json.loads(response.decode("utf-8"))
assert json_body["walletname"] == testfileloc
self.jwt_token = json_body["token"]

def process_lock_response(self, response):
def process_lock_response(self, response, code):
assert code == 200
json_body = json.loads(response.decode("utf-8"))
assert json_body["walletname"] == testfileloc

Expand Down Expand Up @@ -361,7 +409,8 @@ def test_do_coinjoin(self):
yield self.do_request(agent, b"POST", addr, body,
self.process_do_coinjoin_response)

def process_do_coinjoin_response(self, response):
def process_do_coinjoin_response(self, response, code):
assert code == 202
# response code is already checked to be 200
clientconn = self.daemon.coinjoin_connection
# backend's AMP connection must be cleaned up, otherwise
Expand Down

0 comments on commit e6ea9d0

Please sign in to comment.