fix: add coinbase txn id to improve auditability #204
Conversation
| @@ -553,7 +553,8 @@ def _process_transfer( | |||
| ) | |||
| elif transaction_type == _SEND: | |||
| transaction_network = transaction[_NETWORK] | |||
| crypto_hash: str = transaction_network[_HASH] if _HASH in transaction_network else Keyword.UNKNOWN.value | |||
| # although this transaction ID is not global, is improves adutibility in the reports | |||
| crypto_hash: str = transaction_network[_HASH] if _HASH in transaction_network else transaction[_ID] | |||
There was a problem hiding this comment.
Unfortunately doing this is not advisable: using an internal Coinbase id as unique id of an external-facing transaction could confuse the transaction resolver and make it impossible to match the transaction. For external-facing transaction the only two values for unique id are UNKNOWN or the actual hash. For non-resolvable transactions it's OK to use an internal id (because they don't go through the transaction resolver). So I think the approach used in the original code is fine as it is: spell out what to use inside each if branch.
Finally note that for auditability purposes, all the JSON data from the Coinbase REST endpoint is captured in the debug log (including the Coinbase id), so if you needed to lookup a transaction by Coinbase id you could grep in the debug log.
There was a problem hiding this comment.
How can a user looking at the excel/odt docs understand what coinbase ID is referenced in a row/transaction?
That's what I'm trying to solve here
There was a problem hiding this comment.
The best way is to grep the logs, which contain the full JSON. An alternative would be to add the id to the Notes field, but I don't favor this because it's not standardized (meaning that not all plugins do that), so it would be of limited use.
No description provided.