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

coin_movement updates #5019

Merged
merged 4 commits into from
Jan 26, 2022

Conversation

niftynei
Copy link
Collaborator

While working on the accounting plugin, I ran into some problems/limitations of our current event emissions.

  1. We weren't tracking withdrawals to external wallets (initiated via withdraw/multiwithdraw). This makes it really difficult to see where our funds are going, as well as confuses the onchain fee tracker that the accountant plugin has. Now, we emit a coin_movement for

  2. Funds sent to 'external' accounts don't say which account they originated from. While it's possible to figure this out from the utxo tracking, that's sometimes difficult to reconstruct. Instead, for every external deposit, we now record what internal account the payment originated from. For example, wallet payments to an external account will be tagged 'wallet'. Payments to the peer in a channel will 'originate' from the channel's account.

If a coin move concerns an external account, it's really useful to know
which 'internal' account initiated the transfer.

We're about to add a notification for withdrawals, so we can use this to
track wallet pushes to outside addresses

Changelog-Added: JSONRPC: `coin_movement` to 'external' accounts now include an 'originating_account' field
Requested in a comment on a previous PR
Report how much the deposit was for.
The blockheight is zero though, since these aren't included in a block
yet.

We also don't issue an 'external' deposit event if we can tell that the
address you're sending to actually belongs to our wallet (we'll issue a
deposit event when it gets included in a block)
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 050f377

@rustyrussell rustyrussell merged commit 4dafeed into ElementsProject:master Jan 26, 2022
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.

2 participants