View Issue Details

IDProjectCategoryView StatusLast Update
0005558Talerwire pluginspublic2021-09-02 18:14
ReporterMarcello Stanisci Assigned ToMarcello Stanisci  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Product Versiongit (master) 
Target Version0.6Fixed in Version0.6 
Summary0005558: Change "serial id" type to a more abstract one.
DescriptionThe 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*.
TagsNo tags attached.


Christian Grothoff

2019-02-12 20:12

manager   ~0013691

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.

Marcello Stanisci

2019-02-12 22:43

viewer   ~0013694

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?

Christian Grothoff

2019-02-12 23:17

manager   ~0013695

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?

Marcello Stanisci

2019-02-13 16:21

viewer   ~0013703

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?

Christian Grothoff

2019-02-13 16:47

manager   ~0013705

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.

Marcello Stanisci

2019-02-13 19:23

viewer   ~0013710

Fixed in 9ff3c2fce5835dff2100a80f27b.

Marcello Stanisci

2021-09-02 18:14

viewer   ~0018278

Fix committed to master branch.

Related Changesets

exchange: master bb44b9b4

2019-02-13 18:53

Marcello Stanisci

Details Diff
Fix 0005558. Affected Issues
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

Issue History

Date Modified Username Field Change
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