View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0005558||Taler||wire plugins||public||2019-02-11 19:35||2021-09-02 18:14|
|Reporter||Marcello Stanisci||Assigned To||Marcello Stanisci|
|Priority||normal||Severity||minor||Reproducibility||have not tried|
|Product Version||git (master)|
|Target Version||0.6||Fixed in Version||0.6|
|Summary||0005558: Change "serial id" type to a more abstract one.|
|Description||The plugin interface defines the type TALER_WIRE_ConfirmationCallback -- a function that gets called after the bank took our wire transfer command -- to have the "serial_id" argument -- the serial id of the fresh created wire transfer -- as a uint64_t. This is not sufficiently abstract as different banks may have different data to represent "id". We should change this to something like void*, or char*.|
|Tags||No tags attached.|
||Ah, but that one is different. That serial_id is the ID for the transfer in OUR table. It has nothing to do with the row_id of the wire plugin.|
Look at the function execute_cb() @ plugin_wire_taler-bank.c.
This one gets called by handle_admin_add_incoming_finished() @ bank_api_admin.c, that gives it the "serial_id" argument after fetching it from the bank's response ("row_id" field).
execute_cb() then gives this "serial_id" value _verbatim_ to the callback held in "eh->cc", that has the type discussed in this issue (TALER_WIRE_ConfirmationCallback).
So that type *does* take the "row_id" value arrived from the bank.
Do we agree?
execute_cb() gets a 64-bit serial ID because the Taler-bank API mandates 64-bit IDs, that part is fine.
But you are right, the TALER_WIRE_ConfirmationCallback signature is then wrong. There it should again be a "const void *row_id" and a "size_t row_id_size". Also, the 64-bit serial_id should probably be converted to network byte order (NBO), using GNUNET_htonll() first, and then a pointer to the NBO-value should be passed to eh->cc().
Can you just fix this, or does it seem to have larger implications anywhere?
||From a quick view, no, it doesn't seem to have larger implications. Just a question about the NBO: since that row_id value *comes* from the network, isn't it supposed to have *already* a network byte order?|
||Not really, IIRC it came from the network as a JSON to begin with (so in ASCII) and as long as it is a normal variable, we always convert to host byte order. And the API callback of the bank API doesn't specify otherwise, so no, this is (and should be) in host byte order.|
||Fixed in 9ff3c2fce5835dff2100a80f27b.|
||Fix committed to master branch.|
exchange: master bb44b9b4
2019-02-13 17:53:48Details Diff
|mod - src/exchange-tools/taler-wire.c||Diff File|
|mod - src/exchange/taler-exchange-aggregator.c||Diff File|
|mod - src/include/taler_wire_plugin.h||Diff File|
|mod - src/wire-plugins/plugin_wire_taler-bank.c||Diff File|
|mod - src/wire-plugins/test_wire_plugin_transactions_taler-bank.c||Diff File|
|2019-02-11 19:35||Marcello Stanisci||New Issue|
|2019-02-12 20:12||Christian Grothoff||Note Added: 0013691|
|2019-02-12 20:12||Christian Grothoff||Assigned To||=> Marcello Stanisci|
|2019-02-12 20:12||Christian Grothoff||Status||new => feedback|
|2019-02-12 22:43||Marcello Stanisci||Note Added: 0013694|
|2019-02-12 23:17||Christian Grothoff||Note Added: 0013695|
|2019-02-12 23:17||Christian Grothoff||Status||feedback => assigned|
|2019-02-12 23:17||Christian Grothoff||Product Version||=> git (master)|
|2019-02-12 23:17||Christian Grothoff||Target Version||=> 0.6|
|2019-02-13 16:21||Marcello Stanisci||Note Added: 0013703|
|2019-02-13 16:47||Christian Grothoff||Note Added: 0013705|
|2019-02-13 19:23||Marcello Stanisci||Note Added: 0013710|
|2019-02-13 19:24||Marcello Stanisci||Status||assigned => resolved|
|2019-02-13 19:24||Marcello Stanisci||Resolution||open => fixed|
|2019-02-17 22:25||Christian Grothoff||Fixed in Version||=> 0.6|
|2019-12-20 19:12||Christian Grothoff||Status||resolved => closed|
|2021-09-02 18:13||Marcello Stanisci||Changeset attached||=> Taler-exchange master bb44b9b4|
|2021-09-02 18:14||Marcello Stanisci||Note Added: 0018278|