FIO V1 History: Duplicate trnsfiopubky transactions

[ericbutz]

Multiple transfer fio pub key transactions are showing up in the Explorer. For example see: https://explorer.fioprotocol.io/account/o3vozszmxmto

[adsorptionenthalpy]

From @lukestokes

Raw data doesn't show up: https://explorer.fioprotocol.io/transaction/28cf9d4afa8272decc4197bdec4408ac81c73cf619caeed694a97a331911f190

[adsorptionenthalpy]

Transactions seem to be accounted for in the latest history plugin fix. Should be making it into the next release.

For trnsfiopbkey, the following history insertions take place:

action trace inserted by account receiver - entry for the contract account action trace inserted by actor - filtered and inserted if the param exists, action trace is inserted for the actor account action trace inserted by hashed down fio pub key - in this case, the receiver may not exist yet as it was just created before the transfer occurred, additional insertion was created using an account that was derived from the payee public key

The caching of the three separate entries of the same action allows transaction to be searchable by sender, receiver or payee account. So, what we get is all 3 of those action traces when looking up by account. There is a unique action_digest that we will attempt to leverage to filter out the duplicate results. Discussing the next steps with the team...

[adsorptionenthalpy]

Adam A - Dapix, [01.05.20 12:35] Hey @any_x . Seeing some duplication for a some transactions, specifically trnsfiopbky. This is the current modified history plugin. Trying to rule out the plugin itself, could the the explorer.. https://github.com/fioprotocol/fio/blob/feature/MAS-1563_temp/plugins/history_plugin/history_plugin.cpp#L188-L264

Aaron Cox (jesta) — Greymass, [01.05.20 13:01] I believe that's the default behavior, it's returning all traces involving the account being queried. Sadly it doesn't also filter the traces to the account being notified.

Aaron Cox (jesta) — Greymass, [01.05.20 13:03] It can be filtered at the explorer level by receiver though I believe. Those 3 traces being returned have unique receiver fields of:

• fio.token (the contract)
• gjkdwmtx2tse (the originating account)
• l35p5swqgvoo (the destination account)

Adam A - Dapix, [01.05.20 13:12] Makes sense. Filtering out duplicate transactions for the receiver at the explorer should be easy... thanks

Aaron Cox (jesta) — Greymass, [01.05.20 13:15] No problem!

Aaron Cox (jesta) — Greymass, [01.05.20 13:16] Our history viewer in the wallet also suffered from those duplicates, though we never fixed it. I don't remember if there was some reason we didn't do it though, this was a long time ago we looked into it.

Aaron Cox (jesta) — Greymass, [01.05.20 13:17] It's possible that the receiver was incorrect on some of the transactions we acutally wanted to displayed.

Aaron Cox (jesta) — Greymass, [01.05.20 13:17] Just saying all this so you're aware and can make sure going this route doesn't eliminate some sort of data you actually want 😉

Adam A - Dapix, [01.05.20 13:47] Already had a situation where we were storing incorrect account names for address and domain registration actions that weren’t supplied with an owner fio public key. Oops. That’s been cleaned up. 1 duplicate makes sense when you account for both sender and receiver, but 2 duplicates, that’s a little confusing

Adam A - Dapix, [05.05.20 10:36] Hypothetical question related to the latest changes of the history plugin, just throwing it out here. We introduced duplicate results by account_name when doing additional insertions of action traces (which are working great)... if get_actions endpoint were modified to filter out results with identical action digests, what would this harm ?

Adam A - Dapix, [05.05.20 10:43] The actions are exactly the same. If we leave out the extra ones from the result, I only see positives at this point so correct me if wrong.

Adam A - Dapix, [05.05.20 10:47] We could very easily do the filtering on the block explorer or any FR, but dapix product team has concluded I should pursue a clean and compatible solution at the api since we are seeing some noticeable differences in how some of the wallets handle action history.

Adam A - Dapix, [05.05.20 10:48] @jestagram

Aaron Cox (jesta) — Greymass, [05.05.20 12:52] I personally don't know of any issues it'd cause, so long as some transactions don't go missing.

Are we talking about filtering at the API call level or in the actual index/storage level?

@any_x might have a better idea than I do if this would cause any headaches.

Luke Stokes, [05.05.20 13:41] We talked a little yesterday and today about this and I'm wondering if it makes sense to have a call with yourselves, Igor (for Hyperion), what's his name from Amseterdam and his history solution, etc... make sure we are all on the same page as to what should go into the block_log and how that should be indexed for a history solution and how those things should be filtered when showing explorer history.

Luke Stokes, [05.05.20 13:42] the Bloks guys are going to start to working on an integration. We currently have different wallet experiences, from what I hear, regarding edge and trust. I think Edge uses Hyperion, but I'm not sure.

Luke Stokes, [05.05.20 13:42] Should we start this conversation in the EOS Mainnet channel or start a new one?

Aaron Cox (jesta) — Greymass, [05.05.20 13:54] The block_log is independent of this conversation, there's no derived state within it. All transactions will always be in the block_log, the various solutions just have different methods of extracting and generating the derived state from it.

From my point of view there's no much value at the moment getting those parties together. V1 history (native) is different than V2 (Hyperion) in far too many ways and will never be identical, and Chronicles from Amsterdam isn't even an API just a tool to help build datasets for APIs. Each wallet and app independently is going to choose which route they go based on their needs.

I'm willing to engage on the topic, but I don't see what we'd aim to accomplish.

anyx — TeamGreymass, [05.05.20 14:29] Hm, I'm trying to remember... there's two different things this could be; it could be that they are different actions, or it could be that original v1 history implementation did include duplicates. I can't remember off the top of my head if the original v1 did that or not (it was not well implemented).

anyx — TeamGreymass, [05.05.20 14:30] I might have to play with our own history solution and FIO... I know my own solution is correct...

Luke Stokes, [05.05.20 14:30] there was mention of things happening in the chain plugin, so I wanted to make sure this wasn't something that might impact the chain itself

Luke Stokes, [05.05.20 14:31] I also want to make sure if filtering has to happen somewhere in the explore (or in our plugin code) that's clear so that the bloks integration, for example, will work as expected (along with our explorer)

Adam A - Dapix, [05.05.20 14:32] There's nothing altered in the chain plugin for this. What we have are changes introduced by @jestagram that improve the controller and transaction context for FIO support

Adam A - Dapix, [05.05.20 14:32] Maybe you meant chain library

Adam A - Dapix, [05.05.20 14:33] @any_x Here's the current implementation https://github.com/fioprotocol/fio/blob/develop/plugins/history_plugin/history_plugin.cpp#L188-L261

anyx — TeamGreymass, [05.05.20 14:34] Since result there is a set, it leads me to believe these are different ids, so its my first assumption

Adam A - Dapix, [05.05.20 14:38] Same action digest, all 3.

anyx — TeamGreymass, [05.05.20 14:38] Can you give me a recent account that did a trnsfiopubky action?

anyx — TeamGreymass, [05.05.20 14:39] nvm, found one in your example 😜

Adam A - Dapix, [05.05.20 14:39] Here's our test account https://explorer.testnet.fioprotocol.io/account/gjkdwmtx2tse

Adam A - Dapix, [05.05.20 14:40] l35p5swqgvoo recipient account

Adam A - Dapix, [05.05.20 14:41] so we have the same 3 entries if looking up actions by contract, signer or recipient

Adam A - Dapix, [05.05.20 14:41] From my perspective, not an issue

anyx — TeamGreymass, [05.05.20 14:41] Ok, yeah, its my first assumption, it is indeed the correct result. They are 3 separate actions, with different global_action_seq

anyx — TeamGreymass, [05.05.20 14:43] So, quick explanation why this happens: For actions that do alice->contract->bob, there are 3 different actions: The action that alice takes, the action the contract takes, and the action bob takes. In the case of a simple contract like transfer, the actions look identical because there isn't really an interaction there. For more complex contracts that is not necessarily the case.

anyx — TeamGreymass, [05.05.20 14:44] If you inspect the actions directly (e.g. curl fiotestnet.greymass.com/v1/history/get_actions -d '{"account_name":"gjkdwmtx2tse"}' | jq) you can note the subtle differences in the actions themselves. The global_action_seq is the unique identifier of a unique action.

Adam A - Dapix, [05.05.20 14:47] Is that the only subtle difference? Can't we use the action digest to represent a unique identifier instead ?

anyx — TeamGreymass, [05.05.20 14:48] The issue for example with eos is that both the contract and bob can take additional action based on alices action... for example bob could forward the funds along to bobsafe. I guess you don't have this fancy interaction issue all that often in FIO since you're not a smart contract platform, but the point is that the are unique actions

anyx — TeamGreymass, [05.05.20 14:49] the act digest is just the sha of the act struct... let me think

Adam A - Dapix, [05.05.20 14:53] those items in the struct remain the same and represent all pertinent data points about the interacted parties - how much who sent to who

anyx — TeamGreymass, [05.05.20 14:53] Yeah, I think that should work fine for that contract

anyx — TeamGreymass, [05.05.20 14:54] I can't remember how you guys are dealing with ram, but account_ram_deltas could be different on alice or bob's end

Adam A - Dapix, [05.05.20 14:56] That's a good point. Let's see, at this time if its a new emplacement the actor pays ram, but contract "pays" resources for modifications... so I think the account_ram_deltas remain the same payer for transfers

anyx — TeamGreymass, [05.05.20 14:57] Anyways, I guess my long-running point is that this kind of filtering should probably be done at the client/explorer side, since it's tricky to do this at the api/structural level. They are indeed 3 unique actions

Adam A - Dapix, [05.05.20 14:58] Sounds good, that's the feedback I was looking for. Thanks scott

anyx — TeamGreymass, [05.05.20 14:58] Sure thing!

[ericbutz]

We decided that we would look into creating a new get action call for integrators that will filter out duplicate trnsfiopubky calls.

[adsorptionenthalpy]

Eric Butz, [06.05.20 11:38] @any_x, thanks for the info on this. Very helpful. There is still some concern that we have a history node that will return duplicate transactions for our wallet and exchange partners. So we somehow have to tell them how to manually filter FIO transfer transactions. @pawmmm suggested we create a modified get actions call that automatically does the filtering. Any thoughts on that approach?

anyx — TeamGreymass, [13.05.20 15:06] [In reply to Eric Butz] I still highly recommend against it. The format of the contract and its token action structure is identical to all EOSIO chains, so if they see this issue for FIO they are going to see it for EOS Mainnet and everything else. If its a multi-chain wallet (which most are), it seems to me that it is better to identify this commonality and solve it wallet-side. Helping the wallet understand this (as it relates to all EOSIO chains) would be nice of course.

We could likely identify an API-side improvement, but I would argue this is something that should be considered for a V2 spec history. V1 spec history is very blunt and returns a lot of not-so-useful data, it's not great. So, personally, I would suggest not departing further from EOSIO here and having wallets implement different strategies per-eosio chain, and instead help us with V2 spec. ;)

[ericbutz]

Duplicate transaction issue was fixed with https://github.com/fioprotocol/fio/releases/tag/v2.0.0 Closing.