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.

Sign up for GitHub

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

Jump to bottom

Transaction input account annotations not always set properly #668

Closed
jeffomatic opened this issue Mar 2, 2017 · 7 comments
Closed

Transaction input account annotations not always set properly #668

jeffomatic opened this issue Mar 2, 2017 · 7 comments
Assignees
@jbowens
Labels
1.2 bug

Comments

@jeffomatic
Copy link
Contributor

jeffomatic commented Mar 2, 2017 •
edited

UPDATE: This affects the following input annotations:

  • account_id
  • account_alias
  • account_tags
  • is_local

@chrisgarvin originally reported an issue where the API was erroneously returning "is_local": "no" for transaction inputs that are local to the core. #612 was an attempt to address this issue, but it appears to be ongoing. We're now trying to create a reliable repro case.
@jeffomatic jeffomatic added the bug label Mar 2, 2017
@jbowens
Copy link
Contributor

jbowens commented Mar 2, 2017

The problem is that the tx annotating process uses the account_control_programs table for indexing spends. If the ACP expired, it's not guaranteed to still exist despite the account UTXO still being unspent. We can update the tx annotating to use the account_utxos table, but we'll need to ferry along the change flag.

@jbowens
Copy link
Contributor

jbowens commented Mar 2, 2017

Actually, maybe we don't even need the change flag.

@jeffomatic
Copy link
Contributor Author

Ah okay. I'm guessing this actually does more than just cause an incorrect is_local flag...it probably omits the entire account annotation on the input, right? If so I can update the original comment in this issue.

@jbowens
Copy link
Contributor

jbowens commented Mar 2, 2017

yeah, the entire account annotation is omitted

@jbowens
Copy link
Contributor

jbowens commented Mar 2, 2017

We are targeting a fix for 1.1.x, right? But not super high priority?

@jeffomatic jeffomatic changed the title Transaction input's is_local property is not always being set correctly Transaction input account annotations not always set properly Mar 3, 2017
@jeffomatic
Copy link
Contributor Author

@jbowens I think we can deprioritize relative to our immediate bugfix push, but since this is an API bug it would be good to retire it as soon as we get the headroom.

jbowens added a commit that referenced this issue Mar 3, 2017
@jbowens
core/account: keep expired, in-use acps
8b9390d 
This is a short-term fix for #668 within the 1.1.x release branch. In
the future, we should use `account_utxos` to annotate inputs & outputs
instead.
@jbowens jbowens mentioned this issue Mar 3, 2017
Merged
iampogo pushed a commit that referenced this issue Mar 3, 2017
@jbowens @iampogo
core/account: do not delete expired, in-use acps
3e42a33 
Do not delete account control programs that are expired but still hold
assets.

This is a minimal, short-term fix for #668 within the 1.1.x release
branch.
In the future, we should use `account_utxos` to annotate inputs &
outputs instead.

Closes #686
@jbowens
Copy link
Contributor

jbowens commented Mar 3, 2017

Fixed on 1.1-stable but will need a more invasive fix for main

@jbowens jbowens added the 1.2 label Mar 3, 2017
@jbowens jbowens self-assigned this Mar 3, 2017
jbowens added a commit that referenced this issue Mar 3, 2017
@jbowens
core/account: delete spents in separate processor
42da955 
Delete spent account outputs in a separate block processor from the
indexing block processor. This is a prerequisite to removing the `query`
processor's dependency on the `account_control_programs` table.

For #668.

The new ordering of block processor dependencies:
```
account <----+                   + delete expired CPs
             |                   |
             | tx annotator <----+
             |                   |
asset <------+                   + delete spent outputs
```
jbowens added a commit that referenced this issue Mar 3, 2017
@jbowens
core/account: delete spents in separate processor
1a0c82c 
Delete spent account outputs in a separate block processor from the
indexing block processor. This is a prerequisite to removing the `query`
processor's dependency on the `account_control_programs` table.

For #668.

The new ordering of block processor dependencies:
```
account <----+                   + delete expired CPs
             |                   |
             | tx annotator <----+
             |                   |
asset <------+                   + delete spent outputs
```
@jbowens jbowens mentioned this issue Mar 3, 2017
Merged
iampogo pushed a commit that referenced this issue Mar 3, 2017
@jbowens @iampogo
core/account: delete spents in separate processor
5b96c8b 
Delete spent account outputs in a separate block processor from the
indexing block processor. This is a prerequisite to removing the `query`
processor's dependency on the `account_control_programs` table.

For #668.

The new ordering of block processor dependencies:
```
account <----+                   + delete expired CPs
             |                   |
             | tx annotator <----+
             |                   |
asset <------+                   + delete spent outputs
```
iampogo pushed a commit that referenced this issue Mar 3, 2017
@jbowens @iampogo
core/account: delete spents in separate processor
a97b23b 
Delete spent account outputs in a separate block processor from the
indexing block processor. This is a prerequisite to removing the `query`
processor's dependency on the `account_control_programs` table.

For #668.

The new ordering of block processor dependencies:
(account, asset) <- query <- (expire ACPs, delete spent outputs)

Closes #693
jbowens added a commit that referenced this issue Mar 3, 2017
@jbowens
core/account: annotate spend inputs correctly
5ce6f5f 
Correctly annotate spend inputs with account data. When annotating
inputs, determine the source account by looking up the spent output in
`account_utxos` instead of looking up the control program in
`account_control_programs`. The control program isn't guaranteed to
still exist once it's expired.

In a follow up, we should change the output annotating to use
`account_utxos` as well. We'll need to add the `change` flag to
the `account_utxos` table.

Fixes #668.
jbowens added a commit that referenced this issue Mar 3, 2017
@jbowens
core/account: annotate spend inputs correctly
9f706ae 
Correctly annotate spend inputs with account data. When annotating
inputs, determine the source account by looking up the spent output in
`account_utxos` instead of looking up the control program in
`account_control_programs`. The control program isn't guaranteed to
still exist once it's expired.

In a follow up, we should change the output annotating to use
`account_utxos` as well. We'll need to add the `change` flag to
the `account_utxos` table.

Fixes #668.
@jbowens jbowens mentioned this issue Mar 3, 2017
Merged
iampogo pushed a commit that referenced this issue Mar 6, 2017
@jbowens @iampogo
core/account: annotate spend inputs correctly
ea8e952 
Correctly annotate spend inputs with account data. When annotating
inputs, determine the source account by looking up the spent output in
`account_utxos` instead of looking up the control program in
`account_control_programs`. The control program isn't guaranteed to
still exist once it's expired.

In a follow up, we should change the output annotating to use
`account_utxos` as well. We'll need to add the `change` flag to
the `account_utxos` table.

Fixes #668.
iampogo pushed a commit that referenced this issue Mar 6, 2017
@jbowens @iampogo
core/account: annotate spend inputs correctly
0dafbb0 
Correctly annotate spend inputs with account data. When annotating
inputs, determine the source account by looking up the spent output in
`account_utxos` instead of looking up the control program in
`account_control_programs`. The control program isn't guaranteed to
still exist once it's expired.

In a follow up, we should change the output annotating to use
`account_utxos` as well. We'll need to add the `change` flag to
the `account_utxos` table.

Fix #668.

Closes #696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jbowens jbowens

Labels
1.2 bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jeffomatic @jbowens