Skip to content

Refactor transaction tracking to allow for separate address per invoice and more sane event handling - #43

Merged
treeman merged 1 commit into
masterfrom
flowee-cleanup
Jun 4, 2021
Merged

Refactor transaction tracking to allow for separate address per invoice and more sane event handling#43
treeman merged 1 commit into
masterfrom
flowee-cleanup

Conversation

@treeman

@treeman treeman commented May 18, 2021

Copy link
Copy Markdown
Contributor

Closes #9, #26, #27

@treeman

treeman commented May 22, 2021

Copy link
Copy Markdown
Contributor Author

@fstromback I'm having trouble retrieving txid from Flowee, could you help me out a bit here?

I get this from Connection:

%BitPal.Backend.Flowee.Connection.RawMsg{
  data: [
    {9,
     %BitPal.Backend.Flowee.Connection.Binary{
       data: <<31, 130, 166, 200, 190, 225, 71, 52, 156, 43, 185, 148, 193, 163,
         224, 115, 49, 237, 124, 63, 100, 112, 219, 171, 149, 241, 84, 140, 30,
         130, 77, 146>>
     }},
    {6, 1000},
    {8, 1518375},
    {7, 687955}
  ],

But txid that should be tagged 4 is missing.

I tried to record the binary data we got from Flowee and added it as fixtures for unit tests here. I hope that all the data we want is there, which it should if the docs is correct.

You can just do mix test test/backends/flowee/flowee_test.exs to run these specific tests. You can do it on master too, but I've made some minor refactors to the flowee handling in this branch that makes the test fail here (because txid is missing).

@fstromback

Copy link
Copy Markdown
Contributor

Sure, I'll have a look!

@treeman

treeman commented May 25, 2021

Copy link
Copy Markdown
Contributor Author

I just noticed that we do get txid, but only for one of the fixtures...

When a tx is seen it's ok:

on transaction: %{
  address: %BitPal.Backend.Flowee.Connection.Binary{
    data: <<31, 130, 166, 200, 190, 225, 71, 52, 156, 43, 185, 148, 193, 163,
      224, 115, 49, 237, 124, 63, 100, 112, 219, 171, 149, 241, 84, 140, 30,
      130, 77, 146>>
  },
  amount: 1000,
  txid: %BitPal.Backend.Flowee.Connection.Binary{
    data: <<2, 137, 190, 235, 119, 87, 16, 155, 89, 178, 155, 249, 78, 54, 122,
      66, 181, 163, 76, 208, 110, 171, 237, 219, 51, 59, 176, 83, 99, 164, 208,
      229>>
  }
}

But when it's confirmed, no txid:

on transaction: %{
  address: %BitPal.Backend.Flowee.Connection.Binary{
    data: <<31, 130, 166, 200, 190, 225, 71, 52, 156, 43, 185, 148, 193, 163,
      224, 115, 49, 237, 124, 63, 100, 112, 219, 171, 149, 241, 84, 140, 30,
      130, 77, 146>>
  },
  amount: 1000,
  height: 687955,
  offset: 1518375
}

Weird. And when I look at it, the txid isn't in the fixture... But surely we should get that data from flowee?

And another thing: I want to convert txid to strings, something like this: ff416d296da15607ba733ddd80362a1744920658111714913900c8e10a84942d, which we can view in an explorer.

@fstromback

Copy link
Copy Markdown
Contributor

I think I remember seeing this when implementing the interface, now that you mention it. I need to go and have a closer look at what Flowee does to see if this is the intended behavior, or if we need to tell it that we want txids somehow.

Converting them to hex is quite easy, it is just to convert the binary to hex (but reverse the bytes first, I don't know why they differ). I think I have a function for that in the module for bitcoincash addresses.

@treeman

treeman commented May 25, 2021

Copy link
Copy Markdown
Contributor Author

Converting them to hex is quite easy, it is just to convert the binary to hex (but reverse the bytes first, I don't know why they differ). I think I have a function for that in the module for bitcoincash addresses.

Sweet. Now I think the only thing holding this back is the missing txid for the confirmed notification.

@fstromback

Copy link
Copy Markdown
Contributor

I looked at the source code for Flowee to find out what is happening here. Indeed, in the file libs/api/AddressMonitorService.cpp it seems like Flowee does not send the TxId when the transaction has been accepted into a block.

Reading the code I also got a bit confused. It seems like it can return multiple "scripthashed" and "amount" tags at the same time. It seems like a single message might contain multiple transactions (contrary to the documentation), but I need to dig deeper to understand this properly.

If you have time, try (and capture) the following:

  • Subscribe to two addresses.
  • Send three transactions to these: two to the first, and one to the second. This all has to happen before a block has been mined, and all transactions have to end up in the same block.
  • We should get three "accepted into mempool" messages
  • The interesting part is if we get one or three "accepted into block" messages.

I will try to ask about this.

@fstromback

Copy link
Copy Markdown
Contributor

Reported to Flowee, see here

@fstromback

Copy link
Copy Markdown
Contributor

I got an answer from Tom. It seems Flowee is now sending TxId:s. I have not yet tested, but seems good.

Also: I need to re-do some part of that interface, since a transaction may have multiple outputs and we might get multiple outputs from Flowee there. In our case, I think it is just to look for the destination address we are interested in and see if the amount for that one is right.

I will look into this.

@treeman

treeman commented Jun 2, 2021

Copy link
Copy Markdown
Contributor Author

I saw that. Sounds good.

Events are now created to communicate between different parts of the
system, such as subscribing to a specific address and you'll receive
details about a specific transaction.

Invoice tracking follow Stripe's invoice workflow:
https://stripe.com/docs/invoicing/overview#workflow-overview

Also add in fixtures for Flowee, allowing us to test the surrounding
logic without requiring an open connection to a running node.
@treeman
treeman merged commit 8d353fc into master Jun 4, 2021
@treeman
treeman deleted the flowee-cleanup branch June 4, 2021 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generate a new address for each payment request

2 participants