Skip to content

Commit

Permalink
@Gricha - one of the rules of agile programming - don't add placehold…
Browse files Browse the repository at this point in the history
…er classes, don't anticipate. if you don't implement conditions yet, don't create placeholder classes.

reasons for that:
- agile projects often pivot, and you end up with a code full of placeholders that won't get ever used
- placeholders make code harder to read, so there's a recurring cost to having them
- the architecture may change before we get to implementing a given feature, and placeholder might not fit the new architecture

I know we were two steps away from creating the conditions, but still - you should've waited with implementing the condition evaluator until the day you're building the conditions. **and even then** not create a separate class for that (because again - that's anticipating that future conditions will appear), but do it inline first, and once you see the code duplicated, move to create separate functions, and then finally - classes.

If you do it another way (and you are doing it the other way around consistently), you end up with a code filled with empty classes, that's very hard to maintain and modify.
  • Loading branch information
kolinko committed Jul 5, 2014
1 parent 778f8e9 commit 05aff65
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 51 deletions.
3 changes: 0 additions & 3 deletions src/oracle/condition_evaluator/__init__.py

This file was deleted.

7 changes: 0 additions & 7 deletions src/oracle/condition_evaluator/btc_price.py

This file was deleted.

16 changes: 0 additions & 16 deletions src/oracle/condition_evaluator/evaluator.py

This file was deleted.

31 changes: 7 additions & 24 deletions src/oracle/handlers/conditionedtransactionhandler.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from basehandler import BaseHandler
from oracle.condition_evaluator.evaluator import Evaluator
from oracle.oracle_db import SignedTransaction, HandledTransaction, UsedInput
from oracle.oracle_protocol import SUBJECT, RESPONSE

Expand All @@ -18,29 +17,19 @@ class TransactionVerificationError(Exception):
class ConditionedTransactionHandler(BaseHandler):
def __init__(self, oracle):
self.oracle = oracle
self.evaluator = Evaluator()

def handle_task(self, task):
body = json.loads(task['json_data'])
condition = body['condition']
transactions = body['transactions']

permissions_to_sign = self.evaluator.permissions_to_sign(condition, transactions)

if sum(permissions_to_sign) == 0:
logging.debug('no signatures for tx')
self.oracle.task_queue.done(task)
return

for idx, tx in enumerate(transactions):
if permissions_to_sign[idx]:
transaction = tx['raw_transaction']
prevtx = tx['prevtx']
signed_transaction = self.oracle.btc.sign_transaction(transaction, prevtx)
body['transactions'][idx]['raw_transaction'] = signed_transaction
SignedTransaction(self.oracle.db).save({
"hex_transaction": signed_transaction,
"prevtx":json.dumps(prevtx)})
transaction = tx['raw_transaction']
prevtx = tx['prevtx']
signed_transaction = self.oracle.btc.sign_transaction(transaction, prevtx)
body['transactions'][idx]['raw_transaction'] = signed_transaction
SignedTransaction(self.oracle.db).save({
"hex_transaction": signed_transaction,
"prevtx":json.dumps(prevtx)})

self.oracle.communication.broadcast_signed_transaction(json.dumps(body))
self.oracle.task_queue.done(task)
Expand Down Expand Up @@ -177,12 +166,6 @@ def add_transaction(self, message):
logging.debug("locktime must be a number")
return

condition = body['condition']
# Future reference - add parsing condition. Now assumed true
if not self.evaluator.valid(condition):
logging.debug("condition invalid")
return

try:
self.oracle.btc.add_multisig_address(req_sigs, pubkey_list)
except ProtocolError:
Expand Down
2 changes: 1 addition & 1 deletion src/oracle/oracle_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class SUBJECT:
}

OPERATION_REQUIRED_FIELDS = {
OPERATION.TRANSACTION: ['transactions', 'locktime', 'condition', 'pubkey_json', 'req_sigs'],
OPERATION.TRANSACTION: ['transactions', 'locktime', 'pubkey_json', 'req_sigs'],
OPERATION.BOUNTY_CREATE: ['prevtx', 'locktime', 'message_id', 'sum_amount', 'miners_fee', 'oracle_fees', 'pubkey_json', 'req_sigs', 'password_hash', 'return_address'],
OPERATION.GUESS_PASSWORD: ['pwtxid', 'passwords']
}
Expand Down

1 comment on commit 05aff65

@Gricha
Copy link
Contributor

@Gricha Gricha commented on 05aff65 Jul 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kolinko thanks for great lesson. Following that - the transaction should not be named ConditionedTransaction but raather transaction and therefore - there should not be a list pf transactions as you proposed earlier

Please sign in to comment.