Add chain package
#1178
Add chain package
#1178
Conversation
peterjan
force-pushed
the
pj/chain-update-tx
branch
from
db4aa61 to
0b42c54
Compare
peterjan
changed the title
chain package
peterjan
force-pushed
the
pj/chain-update-tx
branch
from
50ed0a3 to
9a6265e
Compare
|
Failing on NDFs in the |
peterjan
force-pushed
the
pj/chain-update-tx
branch
from
b6cfce7 to
15c2659
Compare
peterjan
force-pushed
the
pj/chain-update-tx
branch
from
d06cb6a to
de6679e
Compare
|
@n8maninger would you mind giving this a look? I think Chris def. wants you to have seen this too before we approve it. |
| Address: hash256(e.SiacoinOutput.Address), | ||
| MaturityHeight: e.MaturityHeight, | ||
| Height: index.Height, | ||
| BlockID: hash256(index.ID), |
There was a problem hiding this comment.
This is the same issue we have in walletd right? Where we lose the actual index of the output when performing a reorg.
Leaving this as a reminder to fix it once we have decided what to do with it. Soft deletion via a spent field is probably the best option after all.
There was a problem hiding this comment.
Yeah this impl. now mirrors walletd and I figured we'd change it once we decide on what to do in walletd. Soft deletion is an option but you'd have spent be a block_id I guess. Or a hash of both the id and the height or something. In walletd I'm proposing to just use the index of the block that's being applied instead of the one being reverted?
There was a problem hiding this comment.
We shouldn't mirror walletd for renterd or hostd. I don't believe Siacoin Elements need to be tied to a chain index for these apps. Orphans can't exist since neither app needs to "live" rescan. Delete the elements explicitly in UpdateTx.RevertIndex instead of deleting the index foreign key or cascade deleting.
Associating them with the block being applied creates the same issue if that block ever gets reverted. It is best not to associate them with a chain index at all.
This PR is just an attempt to make the diff smaller on #1098 that's been open since forever, but now this one has been open for weeks. This one's really just trying to shave off ~600 lines off of the diff. I'd like to merge this actually and just fix the way we handle reverts in a F/U. |
This PR adds a
chainpackage that exposes two interfaces implemented by the store. These essentially expose a database transaction, allowing us to atomically apply consensus updates from the subscriber (eventually).Note that the interface is missing
wallet.UpdateTx(and so are the tests), that's because it's a bit of a catch-22. Adding that would require updatingcoreutilsand that would in turn require an update to the current subscriber to make the tests pass. That's not worth the trouble since we are extracting that completely in a F/U.