Add remark to transfers

[gianfra-t]

Issue #30

This PR will add the capability to the indexer of appending remarks that could work as "memos".

The Transfer and TokenTransfer entities now have a new remark field used for this purpose.

AFTER we handle events, we loop through all batch and batchAll calls and match the transfer a user has made with a corresponding remark on the same batch transaction, appending the remark data into the corresponding transfer remark field.

Assumptions

• There exists only one transfer with same amount and destination on that block. Otherwise, only one (latest) would get the remark appended.
• The transfer events must always be processed and stored before we handle the batch calls.
• Each batch and batchAll call must contain a remark and either a Balances.transfer or a Tokens.transfer call. Any other combination of those kinds will be ignored, yet there can be more calls of different kind.

Testing

This implementation was tested using a local chain both with batch and batchAll.

[ebma]

Considering your comment here, we probably should not merge this just yet.

You raised the valid concern that if we have multiple transfers with multiple remarks in the same block originating from the same sender, we are not able to appropriately link remarks and transfers. I think the only way we can have this perfect linking is if we only link these calls if they are contained in a utility::batch or utility::batchAll extrinsic. As far as I know, these extrinsics also emit events, so it should be somewhat easy to move the linking to the handler of those events. What do you think @gianfra-t?

[gianfra-t]

It would be ideal to group the two extrinsics using a batch event, but the event itself does not contain any information about what was executed. As far as a know and also according to this post the only way to do this would be to infer this by the order in which the batch events are executed.

But I am not sure if we can rely on the order 100% of the times when indexing. I will try to look on the subsquid documentation for this. In case we cannot rely on the order, an alternative I can think of would be to wait a block, and include the hash of the transfer into the remark,

[ebma]

Hmm okay, thanks for pointing it out. So I take back that it should be easy to handle the batch calls. But it has to be possible to link all events and calls that happen in a batch extrinsic to exactly that extrinsic. Otherwise polkadot.js also wouldn't be able to do so, e.g. here.

As far as I understand we do have the information about what was executed in a batch transaction because we do know all the calls contained in it.

We would have to find a way to group the calls by their parent extrinsic, but I assume that this information is somehow contained on some attribute on each call entity.

[gianfra-t]

I honestly don't know how Polkadot.js is grouping them. So to be sure the info is there in the event in squids I will try this with a local chain using the utility pallet.

[gianfra-t]

Okay so now I take back what I said haha. Although in the event itself there is no info, we have access to the call that triggered the event. And from the call we can extract both extrinsics!

[gianfra-t]

It actually is deployed as 10, but I realized now the other PR @bogdanS98 is working on is also 10, so I will bump to 11 and wait for that one to merge.

[ebma]

Ah okay, I was not sure which changes the v10 contains. I quickly tried testing it and I submitted this extrinsic on Foucoco but it doesn't seem like the indexer is picking it up, at least I don't see it for this squid with this query

query MyQuery {
  transfers(where: {remark_isNull: false}) {
    remark
    id
  }
  tokenTransfers(where:{remark_isNull:false}) {
    remark
    id
  }
}

[gianfra-t]

That is strange because locally it works. Let me investigate.

I made a transaction and it appears to be working. Strange(er) it looks like your transfer itself is not even registered! (there is not transfer at block 746814)

[ebma]

Strange(er) it looks like your transfer itself is not even registered! (there is not transfer at block 746814)

Ahh wow, that's a nice catch 👍 Then the problem was on my side because I just created a transfer from an account to itself which apparenty did not get executed or at least it didn't emit an event. (Which is smart because a transfer to itself is useless obviously but I did not expect it to get ignored). Nevermind then.

[ebma]

Oh one more thing, do you really want to check in the build artifact or was it by accident? We did not do that so far

[gianfra-t]

Oh one more thing, do you really want to check in the build artifact or was it by accident? We did not do that so far

Accident... sorry. I will remove it before merging!

[ebma]

Alright 👍 since the other PR is merged now we need to resolve the conflicts again

[gianfra-t]

I've deployed v11 so once it syncs I think this is ready.

[gianfra-t]

I commented out the fetching of the max block height because I was getting an error ReferenceError: fetch is not defined at... when deploying the squids. This does not appear when running locally, perhaps fetch is not available on the squids environment.

Let me know what you think @bogdanS98. Did you also get this error?. I think it could still work just by saving the blocks once we reach the latest block. Only downside is that as soon as it starts, the latest blocks will not be available.

[bogdanS98]

@gianfra-t @ebma I only tested locally after adding fetch() and it worked. Global fetch() only works on Node.js versions starting from 18. I use 18.8.2 locally but in the squid environment we have version 16. There are a few options here:

1. We upgrade Node.js version in the squid environment and then we'll have global fetch() available
2. We add node-fetch module and use fetch() from there
3. We use something else instead of fetch but that still requires a module to be imported (e.g. axios)

I would go with the second option but please let me know what you think

[gianfra-t]

I also would go with the second option since fetch already works and upgrading node to 18 may break other things. I will try this later.

[gianfra-t]

We have replaced from using fetch to axios which seems to work on the cloud environment. I will wait for full sync to merge this PR though.