Some refactoring to make possible transactions checking client-side #728

ouziel-slama · 2015-03-05T18:04:11Z

Move sub functions outside get_tx_info2() because counterpary-cli needs the function decode_output() to check transactions generated by a server.

Add raw_unpack_data() method. This method don't use the database or the backend.
It is used by counterparty-client/gui to check transactions generated by counterparty-server.

see: CounterpartyXCP/counterparty-cli#34

adamkrellenstein · 2015-03-05T18:52:01Z

counterpartylib/lib/api.py

+                        param_name = message_module.FORMAT_DESCRIPTION[p]
+                        params[param_name] = unpacked[p]
+                    return params
+    except Exception as e:


This line is no good.

why? here I want to catch all possible errors. No matter what type of error, if there is one, that means that the data is incorrect.

If there's a filesystem read error, e.g., that's not a problem with the unpacking, and it doesn't mean that the data is invalid.

yeah you are right! I will fix that.

ouziel-slama · 2015-03-09T10:34:40Z

I moved transaction checking from counterparty-cli to counterparty-lib. After several attempts I have not managed to use the same functions at the end of transactions.construct(). Checks are too different in the two cases, and IMO wanting to absolutely use the same functions sounds like a solution in search of a problem.

Edit: however we can use the same checking functions in the API: https://github.com/CounterpartyXCP/counterpartyd/pull/728/files#diff-b4466fbb7c2afb413c8537b30b2d4bc7R285

adamkrellenstein · 2015-03-09T12:13:31Z

counterpartylib/lib/transaction.py

+                asset_id = data.pop(asset_param)
+                param_name = asset_param_list[asset_param]
+                try:
+                    data[param_name] = util.generate_asset_name(asset_id, block_index=333501)


Why do we need to try both ways? Shouldn't all newly created transactions use the new asset ids?

hum.. I don't think, for instance we can have a send transaction for an "old" asset.

Ah, right. (I made that mistake before.)

ouziel-slama · 2015-03-09T12:15:53Z

@adamkrellenstein, there is a problem to check btcpay transaction without database: it's impossible to get the address and amount corresponding to the order hash without querying the database.
To enable full client side validation, maybe we should add two mandatories parameters for btcpay: destination and amount.
Or we can show for confirmation to the user, these 2 parameters when the transaction is generated.
thought?

adamkrellenstein · 2015-03-09T12:17:43Z

counterpartylib/lib/transaction.py

+            params['destination'] = params['feed_address']
+
+        # No destination
+        if asked_message_type in ['order', 'dividend', 'broadcast', 'cancel', 'destroy', 'execute', 'publish', 'rps', 'rpsresolve']:


Create new variables parsed_destinations = tx_info['destinations'].keys() and desired_destination = params[destination'], I think.

hum.. seems difficult.. I need yet to well test each transaction.. when done I will give a try.

EDIT: I think the code changed since you message, not sure if I understood well your remark.

adamkrellenstein · 2015-03-09T12:25:07Z

counterpartylib/lib/transaction.py

+            'callable_': False,
+            'divisible': True
+        }
+        for param in data:


This block could be clearer, I think.

I broke the block in several checks.

adamkrellenstein · 2015-03-09T12:30:59Z

You mean mandatory parameters to the create_btcpay call? Yes, I'm fine with adding those.

ouziel-slama · 2015-03-09T12:34:46Z

yep! OK.

adamkrellenstein · 2015-03-12T22:20:29Z

@ouziel-slama, what's the status of this?

ouziel-slama · 2015-03-13T09:14:29Z

@adamkrellenstein, the codebase is finished, but I need to add unit test for every RPC call.

coveralls · 2015-03-15T23:37:58Z

Coverage increased (+0.48%) to 69.34% when pulling 88079ec on checktx into 9c0e0f2 on develop.

coveralls · 2015-03-15T23:53:40Z

Coverage increased (+0.48%) to 69.34% when pulling 9e6ddea on checktx into 9c0e0f2 on develop.

ouziel-slama · 2015-03-16T10:12:59Z

@adamkrellenstein, there is now enough tests for RPC calls... however the kickstart failed, not sure it's linked with this PR, but I will check that tonight.

`counterpary-cli` needs the function `decode_output()` to check transactions generated by a server.

This method don't use the database or the backend. It used by `counterparty-client/gui` to check generated transaction generated by `counterparty-server`.

…d `destination` enable client side validation of generated transactions

coveralls · 2015-04-01T23:47:10Z

Coverage increased (+0.58%) to 65.93% when pulling 38c5946 on checktx into 7076bc0 on develop.

ouziel-slama · 2015-04-02T09:58:55Z

OK, I rebased the branch and the kickstart works well.. don't know what happen the last time I tried..

adamkrellenstein · 2015-04-02T12:54:03Z

@ouziel-slama, why/how did the kickstart fail? Are we sure it won't be an intermittent problem?

ouziel-slama · 2015-04-03T16:26:00Z

@adamkrellenstein: the parsing didn't stop at BLOCK_START, he continued until block 1.. I don't understand how it's happened (that mean that the kickstart just skip the first block, see here: https://github.com/CounterpartyXCP/counterpartyd/blob/master/counterpartylib/lib/blocks.py#L835)
I ran two kickstarts without problems.. I will launch more to be sure.

adamkrellenstein · 2015-04-03T16:49:34Z

Created this issue #749.

coveralls · 2015-04-10T12:10:54Z

Coverage decreased (-0.54%) to 64.85% when pulling 38c5946 on checktx into a4225b0 on develop.

chiguireitor · 2018-09-26T00:58:54Z

This Pr although useful, has diverged too much from the codebase, closing this and saving on branch pr728

adamkrellenstein reviewed Mar 5, 2015
View reviewed changes

ouziel-slama force-pushed the checktx branch 2 times, most recently from 5aef8a3 to cfa69bd Compare March 9, 2015 12:03

adamkrellenstein reviewed Mar 9, 2015
View reviewed changes

ouziel-slama force-pushed the checktx branch from cfa69bd to f5d4fd5 Compare March 9, 2015 12:23

adamkrellenstein reviewed Mar 9, 2015
View reviewed changes

ouziel-slama force-pushed the checktx branch from f5d4fd5 to 5509666 Compare March 9, 2015 12:33

ouziel-slama force-pushed the checktx branch 2 times, most recently from 1eb272d to b61f3ca Compare March 9, 2015 15:49

ouziel-slama force-pushed the checktx branch from 88079ec to 9e6ddea Compare March 15, 2015 23:47

ouziel-slama force-pushed the develop branch from dbd32e7 to 55674d6 Compare March 18, 2015 10:19

ouziel-slama added 3 commits April 1, 2015 23:26

Move sub functions outside get_tx_info2()

8613b94

`counterpary-cli` needs the function `decode_output()` to check transactions generated by a server.

Add raw_unpack_data() method

525a750

This method don't use the database or the backend. It used by `counterparty-client/gui` to check generated transaction generated by `counterparty-server`.

Check outputs transactions without database or backend

2241a67

ouziel-slama added 4 commits April 1, 2015 23:30

add two required parameters for create_btcpay method: quantity an…

347c71f

…d `destination` enable client side validation of generated transactions

Complete and refactor data formating before comparaison with parameters

d122cee

More API test and fixes

9ebd6cb

Fix rebase

38c5946

ouziel-slama force-pushed the checktx branch from 9e6ddea to 38c5946 Compare April 1, 2015 23:41

adamkrellenstein closed this Apr 3, 2015

ouziel-slama reopened this Apr 3, 2015

ouziel-slama force-pushed the checktx branch 2 times, most recently from 5195971 to 38c5946 Compare April 10, 2015 12:07

chiguireitor closed this Sep 26, 2018

adamkrellenstein deleted the checktx branch January 21, 2024 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some refactoring to make possible transactions checking client-side #728

Some refactoring to make possible transactions checking client-side #728

ouziel-slama commented Mar 5, 2015

adamkrellenstein Mar 5, 2015

ouziel-slama Mar 5, 2015

adamkrellenstein Mar 5, 2015

ouziel-slama Mar 5, 2015

ouziel-slama commented Mar 9, 2015

adamkrellenstein Mar 9, 2015

ouziel-slama Mar 9, 2015

adamkrellenstein Mar 9, 2015

ouziel-slama commented Mar 9, 2015

adamkrellenstein Mar 9, 2015

ouziel-slama Mar 9, 2015

ouziel-slama Mar 9, 2015

adamkrellenstein Mar 9, 2015

ouziel-slama Mar 9, 2015

adamkrellenstein commented Mar 9, 2015

ouziel-slama commented Mar 9, 2015

adamkrellenstein commented Mar 12, 2015

ouziel-slama commented Mar 13, 2015

coveralls commented Mar 15, 2015

coveralls commented Mar 15, 2015

ouziel-slama commented Mar 16, 2015

coveralls commented Apr 1, 2015

ouziel-slama commented Apr 2, 2015

adamkrellenstein commented Apr 2, 2015

ouziel-slama commented Apr 3, 2015

adamkrellenstein commented Apr 3, 2015

coveralls commented Apr 10, 2015

chiguireitor commented Sep 26, 2018

Some refactoring to make possible transactions checking client-side #728

Some refactoring to make possible transactions checking client-side #728

Conversation

ouziel-slama commented Mar 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouziel-slama commented Mar 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouziel-slama commented Mar 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamkrellenstein commented Mar 9, 2015

ouziel-slama commented Mar 9, 2015

adamkrellenstein commented Mar 12, 2015

ouziel-slama commented Mar 13, 2015

coveralls commented Mar 15, 2015

coveralls commented Mar 15, 2015

ouziel-slama commented Mar 16, 2015

coveralls commented Apr 1, 2015

ouziel-slama commented Apr 2, 2015

adamkrellenstein commented Apr 2, 2015

ouziel-slama commented Apr 3, 2015

adamkrellenstein commented Apr 3, 2015

coveralls commented Apr 10, 2015

chiguireitor commented Sep 26, 2018