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

tarofreighter: add new ExportLog interface for committing spends #114

Merged
merged 3 commits into from
Sep 17, 2022

Conversation

Roasbeef
Copy link
Member

In this PR, we add a new ExportLog interface that'll be used to
update the on-disk information related to which assets are spent and how
they're spent.

When an asset is spent, all assets that were collocated in the same
commitment must now be re-anchored in a new commitment. However some
assets will also be mutated if they were used to create a split, or mark
a complete spend/send to a party (interactive or non-interactive).

A spend moves from pending to confirmed once it's sufficiently buried in
the chain. At this point, the on-disk information that tracks where the
chain_txn is located will be updated.

At this time, roll backs aren't supported (re-org or need to undo a
spend). As a result, a spend should only be marked as pending once it's
certain that the transactions that anchors the new assets will
eventually be broadcaster and confirmed.

Depends on #113

@Roasbeef
Copy link
Member Author

Ok this is ready for review now.

@Roasbeef Roasbeef requested review from bhandras, jharveyb and guggero and removed request for bhandras September 13, 2022 04:53
@Roasbeef
Copy link
Member Author

Only the last 3 commits are new.

@Roasbeef
Copy link
Member Author

So I think it makes sense to just add an intermediate table here (described in a TODO). This'll let us retain an internal history for all transfer, which can be used to create a CLI/RPC command to browse the history.

The only change needed here is to not apply the update immediately, but instead insert the change/delta, then only apply it once confirmed. We don't need to get down the format 100% rn, but this'll also eventually thread into the CLI output after a send.

Brainstorming a bit, minimally we want to output:

  • the old anchor point + taro root
  • the new anchor point + taro root
  • the bitcoin transaction used for the transfer
  • taro level transfer info (can kinda mirror the JSON format we expect today):
    • the prev ID for each input, along w/ the amt and script key
    • the new set of outputs, and any split commit info for that output

Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Very nice to see yet another component falling into place 🎉

Main question is about differentiating between assets under our control vs. those that were sent out to another entity.

tarodb/assets_store.go Show resolved Hide resolved
tarodb/assets_store.go Show resolved Hide resolved
Copy link
Collaborator

@jharveyb jharveyb left a comment

Choose a reason for hiding this comment

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

Looks good, neat to see the update process materialize 🚀

Some comments but may all be out of scope / non-blocking.

// AssetSpendDelta describes the mutation of an asset as part of an outbound
// parcel (batched send). As we always require script keys to be unique, we
// simply need to know the old script key, and the new amount.
type AssetSpendDelta struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think with this format, we can't represent complete removal of an asset leaf? Once we resolve #121 this wouldn't be needed for normal spends, but may also be relevant for recording asset burns. Maybe out of scope for this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, this assumes the happy case of single input, and the asset amount is sized juuust right.

I think to represent a leaf being removed all together, we'd either use a diff delta struct, or add something here that says it can just be deleted.

tarofreighter/interface.go Outdated Show resolved Hide resolved
tarodb/assets_store.go Show resolved Hide resolved
tarodb/assets_store_test.go Outdated Show resolved Hide resolved
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Needs a rebase then this is good to go 🎉

In this commit, we add a new `ExportLog` interface that'll be used to
update the on-disk information related to which assets are spent and how
they're spent.

When an asset is spent, all assets that were collocated in the same
commitment must now be re-anchored in a new commitment. However _some_
assets will also be mutated if they were used to create a split, or mark
a complete spend/send to a party (interactive or non-interactive).

A spend moves from pending to confirmed once it's sufficiently buried in
the chain. At this point, the on-disk information that tracks where the
chain_txn is located will be updated.

At this time, roll backs aren't supported (re-org or need to undo a
spend). As a result, a spend should only be marked as pending once it's
certain that the transactions that anchors the new assets will
eventually be broadcaster and confirmed.
In this commit, we update the set of on disk asset deltas to only be a
delta. Once we confirm the transaction, then we'll actually apply it.
This allows us to do things like get the set of unconfirmed deltas to
rebroadcast later.
@Roasbeef Roasbeef merged commit d0d95b0 into main Sep 17, 2022
@guggero guggero deleted the asset-re-anchor branch September 17, 2022 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants