Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: event queue connection class design #71

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

r4mmer
Copy link
Member

@r4mmer r4mmer commented Dec 12, 2023

@r4mmer r4mmer requested review from andreabadesso and removed request for glevco December 22, 2023 16:14
Copy link
Contributor

@andreabadesso andreabadesso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be great to be more explicit on how the specific events from the reliable integrations feed are going to be handled, like NEW_VERTEX_ACCEPTED and VERTEX_METADATA_CHANGED

1. Try to find the transaction with the input's `tx_id` in storage
1. If not found we must fetch the tx from the fullnode api
2. Assign `value`, `token_data`, `script` and `token` from the spent output
3. Derive `decoded` from the script.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fullnode already sends it decoded, the documentation might not have been updated because this was a change I requested, here is an example input:

{
  "tx_id": "00000000000000a7880484eb5f059e7857973d7c4d131a9bd59389afe6b0dfc6",
  "index": 0,
  "spent_output": {
    "value": 800,
    "token_data": 0,
    "script": "dqkUtlo02wdlNsUJx3OADdSG/uaHGnWIrA==",
    "decoded": {
      "type": "P2PKH",
      "address": "HP9KRutTVPPf5h8jZ3BMwfhkJoZpD1c7if",
      "timelock": null
    }
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.


#### Events

- `new-tx` and `update-tx`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semi-unrelated to line number, but remember to add a ping/pong mechanism

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document of the event queue and the implementation did not have ping/pong anywhere but from what I understand it comes with the websocket lib we use, so I'll add the ping/pong to the design

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autobahn answers the PING frame with a PONG and also disconnects when no messages are received in a window. It also doesn't send a PING, so the implementation must be done in the client

we need to check if the fullnode we are connected to is the same and if it isn't
we will need to re-sync as usual.

To make sure we are connected to the same fullnode we will use the peer-id, an
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to assert that the stream_id is the same from the last sync

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.

When unvoiding a transaction we can treat it as a new transaction since its
effect on the wallet was already removed when it was voided.

If the `voided_by` field is the same we can iterate on the outputs and calculate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If the `voided_by` field is the same we can iterate on the outputs and calculate
If the `voided_by` field is empty we can iterate on the outputs and calculate

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used null instead of empty but to the same effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants