View Issue Details

IDProjectCategoryView StatusLast Update
0005558Talerobsolete componentpublic2023-12-03 00:22
ReporterMarcello Stanisci Assigned ToMarcello Stanisci Resolution deadline
ImplementerReviewer 
Release signed byDeployed by 
VerifierCommunicator 
Data protection authority notification requiredNo  
CSIRT notification requiredNo  
Estimated effort (hours)Actual effort (hours)0 
Fast-trackNo
Work packagenone
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.

Required notifications

Address or linkRemarkAdded byNotifiedActions

Activities

Christian Grothoff

2019-02-12 19: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 21:43

reporter   ~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 22: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 15:21

reporter   ~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 15: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 18:23

reporter   ~0013710

Fixed in 9ff3c2fce5835dff2100a80f27b.

Marcello Stanisci

2021-09-02 16:14

reporter   ~0018278

Fix committed to master branch.

Issue History

Date ModifiedUsernameFieldChange
2019-02-11 18:35Marcello StanisciNew Issue
2019-02-12 19:12Christian GrothoffNote Added: 0013691
2019-02-12 19:12Christian GrothoffAssigned To => Marcello Stanisci
2019-02-12 19:12Christian GrothoffStatusnew => feedback
2019-02-12 21:43Marcello StanisciNote Added: 0013694
2019-02-12 22:17Christian GrothoffNote Added: 0013695
2019-02-12 22:17Christian GrothoffStatusfeedback => assigned
2019-02-12 22:17Christian GrothoffProduct Version => git (master)
2019-02-12 22:17Christian GrothoffTarget Version => 0.6
2019-02-13 15:21Marcello StanisciNote Added: 0013703
2019-02-13 15:47Christian GrothoffNote Added: 0013705
2019-02-13 18:23Marcello StanisciNote Added: 0013710
2019-02-13 18:24Marcello StanisciStatusassigned => informing
2019-02-13 18:24Marcello StanisciResolutionopen => fixed
2019-02-17 21:25Christian GrothoffFixed in Version => 0.6
2019-12-20 18:12Christian GrothoffStatusinforming => closed
2021-09-02 16:13Marcello Stanisci
2021-09-02 16:14Marcello StanisciNote Added: 0018278
2023-12-03 00:22Christian GrothoffCategorywire plugins => py bank (demonstrator, obsolete)
2023-12-03 00:23Christian GrothoffCategorypy bank (demonstrator, obsolete) => obsolete componet
2023-12-11 19:08Florian DoldCategoryobsolete componet => obsolete component