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

multi: list asset transfers through RPC + cli #158

Merged
merged 8 commits into from
Sep 29, 2022
Merged

Conversation

bhandras
Copy link
Member

This PR adds a new RPC and the matching cli command to list pending and past transfers. (repoen of #135 after the repo went public).

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.

LGTM 🎉

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🥟

return err
}
// Keep the old proofs as a reference for when we list past
// transfers.
Copy link
Member

Choose a reason for hiding this comment

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

Still getting through the the diff, but as is we might end up storing slightly redundant data, since these are the proof suffixes (new state transitions). This would only really be for the sender though, since the receiver's aren't stored in the DB though.


// QueryParcels returns the set of confirmed or unconformed parcels.
func (a *AssetStore) QueryParcels(ctx context.Context,
pending bool) ([]*tarofreighter.OutboundParcelDelta, error) {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -936,6 +937,8 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {
},
},
TapscriptSibling: currentPkg.InputAsset.TapscriptSibling,
// TODO(bhandras): use clock.Clock instead.
TransferTime: time.Now(),
Copy link
Member

Choose a reason for hiding this comment

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

Good move to also move this into the application layer so the DB doesn't explicitly need to worry about time itself.


// The new Taro root that commits to the set of modified and unmodified
// assets.
bytes taro_root = 4;
Copy link
Member

Choose a reason for hiding this comment

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

Totally non-blocking, but I'm thinking we also want to start showing the branch hash and also value here. Then people can spot check the roots to see things like the total number of units, etc.

repeated AssetSpendDelta asset_spend_deltas = 6;
}

message AssetSpendDelta {
Copy link
Member

Choose a reason for hiding this comment

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

In the future I think we also want to add more information here like splits, etc -- but this is a solid base to build off of.

@Roasbeef Roasbeef merged commit 0bae4b6 into main Sep 29, 2022
@guggero guggero deleted the list-transfers branch September 29, 2022 21:34
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.

None yet

3 participants