jharveyb
commented
2 weeks ago I have a similar recollection re: empty responses for REST clients.
On a fresh look, I'm not sure why we don't have a nil-ness check here before trying to unmarshal?
It seems like the 'more-correct' option there would be to return something like universe.ErrNoUniverseRoot instead of unmarshalling something empty. Or skip unmarshalling and return an empty universe.Root{}.
In that case, I think we'd want to extend executeSync to validate the contents of targetRoots before actually executing the sync, and return a more useful error re: missing roots on the target server for the sync.
I'm not sure if other users of the sync interface like the scheduled sync rely on that unmarshal failing as a signal of a missing root though.
dstadulis
Background
Today
QueryAssetRootswill return an empty error when no asset root can be found: https://github.com/lightninglabs/taproot-assets/blob/2d8d4849df66c9632038018a8da01a882be39baf/rpcserver.go#L4707-L4710If a user attempts to query for a non-existent asset ID, the'll get an error that:
Which is confusing, as the real error lies in the call to unmarshall the empty root: https://github.com/lightninglabs/taproot-assets/blob/2d8d4849df66c9632038018a8da01a882be39baf/universe_rpc_diff.go#L120
We'll find that there's no asset ID as no real root was returned: https://github.com/lightninglabs/taproot-assets/blob/2d8d4849df66c9632038018a8da01a882be39baf/universe_rpc_diff.go#L44-L56
Historically, I think this behavior exists to just return an empty response rather than error out for REST clients? Either way, the CLI should be able to properly detect that no roots are found. This may have some implications to the sync and auto syncer in the
universepackage.Steps to reproduce
Run
tapcli universe sync --asset_id=Xwith an asset ID that doesn't exist.Expected behavior
CLI says the asset ID doesn't exist on the universe.
Actual behavior
Obscure decoding error.