Merge pull request #142 from devin-fisher/fix_TOK_387-2

don't allow multiple outputs on TXN fees

Author: mac-arrap

sovtoken/sovtoken/test/helpers/helper_request.py

@@ -1,6 +1,6 @@
 import json
 
-from indy.ledger import build_nym_request
+from indy.ledger import build_nym_request, build_schema_request
 from plenum.common.constants import TXN_TYPE, CURRENT_PROTOCOL_VERSION, GET_TXN, DATA
 from plenum.common.request import Request
 from plenum.common.types import f
@@ -104,8 +104,7 @@ def nym(
         sdk_wallet=None,
     ):
         """ Builds a nym request. """
-        sdk_wallet = sdk_wallet or self._steward_wallet
-        _, sdk_wallet_did = sdk_wallet
+        sdk_wallet_did = self._find_wallet_did(sdk_wallet)
 
         if not dest:
             (dest, new_verkey) = self._wallet.create_did(
@@ -129,6 +128,23 @@ def nym(
 
         return request
 
+    def schema(
+            self,
+            schema_data,
+            sdk_wallet=None
+    ):
+        sdk_wallet_did = self._find_wallet_did(sdk_wallet)
+        schema_request_future = build_schema_request(sdk_wallet_did, schema_data)
+        schema_request = self._looper.loop.run_until_complete(schema_request_future)
+        request = self._sdk.sdk_json_to_request_object(json.loads(schema_request))
+        request = self._sign_sdk(request, sdk_wallet=sdk_wallet)
+        return request
+
+    def _find_wallet_did(self, sdk_wallet):
+        sdk_wallet = sdk_wallet or self._steward_wallet
+        _, sdk_wallet_did = sdk_wallet
+        return sdk_wallet_did
+
     def payment_signatures(self, inputs, outputs):
         """ Generate a list of payment signatures from inptus and outputs. """
         signatures = []

sovtoken/sovtoken/token_req_handler.py

@@ -87,8 +87,8 @@ def validate_given_inputs_outputs(inputs_sum, outputs_sum, required_amount, requ
         if inputs_sum == required_amount:
             return  # Equal is valid
         elif inputs_sum > required_amount:
-            error = 'Extra funds, sum of inputs is {}' \
-                    'but required amount is {}. sum of outputs: {}'.format(inputs_sum, required_amount, outputs_sum)
+            error = 'Extra funds, sum of inputs is {} ' \
+                    'but required amount: {} -- sum of outputs: {}'.format(inputs_sum, required_amount, outputs_sum)
             if error_msg_suffix and isinstance(error_msg_suffix, str):
                 error += ' ' + error_msg_suffix
             raise ExtraFundsError(getattr(request, f.IDENTIFIER.nm, None),

sovtokenfees/sovtokenfees/static_fee_req_handler.py

@@ -30,6 +30,8 @@
 
 logger = getlogger()
 
+MAX_FEE_OUTPUTS = 1
+
 
 class StaticFeesReqHandler(FeeReqHandler):
     valid_txn_types = {SET_FEES, GET_FEES, FEE_TXN}
@@ -95,6 +97,15 @@ def can_pay_fees(self, request):
             self._get_deducted_fees_xfer(request, required_fees)
             self.deducted_fees_xfer[request.key] = required_fees
         elif required_fees:
+            # We don't want to allow transfers on txn fees. So only one OUTPUT address can be used.
+            # We could consider lock this down even more by requiring OUTPUT address to be one of the
+            # INPUT address
+            outputs = request.fees[1]
+            if len(outputs) > MAX_FEE_OUTPUTS:
+                raise InvalidClientRequest(request.identifier,
+                                           request.reqId,
+                                           "Only {} OUTPUT is allow for Transaction fees".format(MAX_FEE_OUTPUTS))
+
             self._get_deducted_fees_non_xfer(request, required_fees)
 
     # TODO: Fix this to match signature of `FeeReqHandler` and extract
@@ -144,12 +155,12 @@ def doStaticValidation(self, request: Request):
         else:
             super().doStaticValidation(request)
 
-    def validate(self, req: Request):
-        operation = req.operation
+    def validate(self, request: Request):
+        operation = request.operation
         if operation[TXN_TYPE] == SET_FEES:
-            validate_multi_sig_txn(req, TRUSTEE, self.domain_state, self.MinSendersForFees)
+            validate_multi_sig_txn(request, TRUSTEE, self.domain_state, self.MinSendersForFees)
         else:
-            super().validate(req)
+            super().validate(request)
 
     def get_query_response(self, request: Request):
         return self.query_handlers[request.operation[TXN_TYPE]](request)
@@ -307,8 +318,8 @@ def _handle_incorrect_funds(sum_inputs, sum_outputs, expected_amount, required_f
             raise InsufficientFundsError(request.identifier, request.reqId, error)
         if sum_inputs > expected_amount:
             error = 'Extra funds, sum of inputs is {} ' \
-                    'but required is {} (sum of outputs: {}, ' \
-                    'fees: {})'.format(sum_inputs, expected_amount, sum_outputs, required_fees)
+                    'but required is: {} -- sum of outputs: {} ' \
+                    '-- fees: {})'.format(sum_inputs, expected_amount, sum_outputs, required_fees)
             raise ExtraFundsError(request.identifier, request.reqId, error)
 
     @staticmethod

sovtokenfees/sovtokenfees/test/helpers/helper_request.py

@@ -4,6 +4,9 @@
 from sovtokenfees.constants import SET_FEES, FEES, GET_FEES
 from sovtoken.constants import AMOUNT, ADDRESS
 
+from stp_core.common.log import getlogger
+
+logger = getlogger()
 
 class HelperRequest(token_helper_request.HelperRequest):
     """
@@ -56,10 +59,21 @@ def add_fees(
         change = total_inputs - fee_amount
 
         if change > 0 and change_address:
-            outputs = [{ADDRESS: change_address, AMOUNT: change}]
+            if not isinstance(change_address, list):
+                change_address = [change_address]
+            change_part = change // len(change_address)
+            if change_part <= 0:
+                raise Exception("Unable to divide {}, {} ways".format(change, len(change_address)))
+            change_mod = change % len(change_address)
+            outputs = []
+            for address in change_address:
+                outputs.append({ADDRESS: address, AMOUNT: change_part+change_mod})
+                change_mod = 0 # Only add remainder once
         else:
             outputs = []
 
+        logger.info("*"*20)
+        logger.info(str(outputs))
         fees_signatures = self.fees_signatures(inputs, outputs, request.digest)
 
         inputs = self._prepare_inputs(inputs)

sovtokenfees/sovtokenfees/test/test_fees_non_xfer_txn.py

@@ -5,7 +5,8 @@
 from plenum.common.exceptions import RequestRejectedException, RequestNackedException
 from plenum.common.txn_util import get_seq_no, get_payload_data, get_txn_time
 from plenum.common.types import f
-from plenum.test.pool_transactions.helper import sdk_build_get_txn_request
+
+from sovtoken.test.wallet import Address
 from sovtokenfees.constants import FEES, REF
 from sovtoken import TOKEN_LEDGER_ID
 from sovtoken.constants import INPUTS, OUTPUTS, AMOUNT, ADDRESS, SEQNO
@@ -130,6 +131,29 @@ def test_fees_larger(
         helpers.sdk.send_and_check_request_objects([req])
 
 
+def test_fees_too_many_outputs(
+    helpers,
+    fees_set,
+    address_main,
+    mint_tokens,
+):
+    """
+    More than one output adress
+    """
+    req = helpers.request.nym()
+    fee_amount = fees_set[FEES][req.operation[TXN_TYPE]]
+    utxos = helpers.general.get_utxo_addresses([address_main])[0]
+    helpers.request.add_fees(
+        req,
+        utxos,
+        fee_amount,
+        change_address=[address_main, Address().address]
+    )
+    with pytest.raises(RequestRejectedException):
+        helpers.sdk.send_and_check_request_objects([req])
+
+
+
 def test_fees_incorrect_sig(
     helpers,
     fees_set,