multi: store optional key tweak in assets table and tweak script keys BIP0086 style #125
Conversation
|
Itests failing due to a mismatch w/ the on disk indexes, commented here: https://github.com/lightninglabs/taro/pull/112/files#r973646487 The issue is that we're using the tweaked key externally, but only storing the raw key on the DB. All the queries are then related to the raw key, so when we go to export by tweaked key that fails. I think we also need to add the tweaked key itself into the Alternatively, we store the tweaked key itself along side the |
|
Alternatively, we can return both the normal and tweaked key on the RPC interface, then the test can use this value to look up the proof file. |
bhandras
force-pushed
the
bip86-keys-fixes
branch
from
4210ab3
to
ab3814f
Compare
|
Quickly fixed the linter error, plus the last commit is now squashed. Originally I removed that |
Fixes bug where the proof file insert would fail since we're using the raw script key in a join clause within `UpsertAssetProof`.
In this commit, we add a new struct to house the tweaked script key information. This new struct lets us leave the old scriptkey.pubkey as the main key we always use to encode and also verify signatures against. The new struct is then embedded within the ScriptKey struct as a pointer, so anytime we're comparing to wire assets, it'll match up properly.
In this commit, we move the script key tweak to live inside the asset rather than within the internal key. This matches the existing managed utxo struct that contains the tweak (sibling and taro root) inside the row, which is then used to construct the actual key that the managed UTXO commits to. We also do this for the address as well. In the future, this'll allow us to use the tweak as a foreign key in another row that contains the actual scripts we're committing to.
This test fails as is, since it relies on using the actual script key instead of the raw script key to fetch the proof on disk. The next commit will resolve this by storing both the tweaked key and the regular key similar to the way we handle key families on disk.
In this commit, we modify things to use a distinct table for script_keys. Moving to this surfaced a bug for which a test case was added in the past commit. This move mirrors the family keys where we store the tweaked key and the raw key, along with all the items we need to re-derive the key. This move also surfaced an issue in the existing itests which import a proof right at the very end: we only have the tweaked script key, so the import just mirrors the state and doesn't actually let us spend the assets. For the addr send case, we've already created the actual script key, so we can use the tweaked key in the new asset to find the actual ID we need.
Roasbeef
force-pushed
the
bip86-keys-fixes
branch
from
ab3814f
to
ca38ea9
Compare
|
So ended up just rolling things into a new Everything works e2e now, though there's a bit of a hack in place to allow the existing itest to work where we mirror the state of another node. Before we had the raw pubkey and just inserted that with no real key desc information. We can't do that now since we have a tweaked key (or it could be w/e) so that test is just good for mirroring the state of one node to another. I think the best way to review this is the full diff vs the individual commits. |
There was a problem hiding this comment.
Nice work, both of you! This should make it pretty easy to enable scripts for assets as well 🎉
Found some small things as well, but will merge this PR to unblock all of us and fix the nits and smaller things in #117 (which I think is pretty close to be merged as well).
| @@ -821,6 +831,8 @@ func (a *AssetStore) importAssetFromProof(ctx context.Context, | |||
|
|
|||
| // Next, we'll insert the genesis_asset row which holds the base | |||
| // information for this asset. | |||
| // | |||
| // TODO(roasbeef): should be an upsert | |||
There was a problem hiding this comment.
I have a commit in one of my PRs, will remove the TODO there.
This commit fixes most of the review comments in #125.
Replaces #112
Rebased on top of master. The best way to review this is the entire diff since I've retained all the intermediate commits to ensure proper authorship.
The main diff between this and #112 is that everywhere here
ScriptKey.PubKey(which is justbtcec.PublicKeyis just the right key. We only reach into the full key desc when we need to go sign, etc .Fixes #94