taproot-assets icon indicating copy to clipboard operation
taproot-assets copied to clipboard

[feature]: Exercise asset locktimes

Open jharveyb opened this issue 10 months ago • 7 comments

As part of asset channel support, we'd like to start using the asset-level locktimes (even though all the anchor transactions will be using locktimes as they are now for LN).

Specifically, we'd like to:

  • Extract the locktimes of the anchor TX for asset inputs
  • Have the VM check these locktimes when transferring an asset
  • Plumb in locktime support for the virtual transactions we use to represent transfers
  • Add assertions around all output locktimes being shorter than the transfer anchor TX locktimes (related to re-org safety)
  • Ensure that these locktimes are also checked during proof verification

Some related pre-existing code that would be useful for this:

https://pkg.go.dev/github.com/btcsuite/btcd/blockchain#IsFinalizedTransaction

https://pkg.go.dev/github.com/btcsuite/btcd/blockchain#SequenceLockActive

I think we could work 'inside-out' to implement this, so:

  • Wire up locktime support when constructing virtual TXs (& then validating)
  • Add args to the VM to pass in the current block height, etc.
  • Wire up locktime propagation when constructing virtual TXs for transfer
  • Add locktime checking to coin selection at the beginning of the transfer logic
  • Add args for the transfer flow (PSBT only?) to add locktimes to outputs
  • Update proof verifier to propagate anchor locktimes and info all the way to the VM

jharveyb avatar Apr 17 '24 21:04 jharveyb

Considering the fact that we are talking about assets in a lightning channel, what happens if we don't support locktimes for in-channel asset transfers? You still have the locktimes of the anchor tx, so there is no chance of spending an asset before said locktime. The re-org vulnerability doesn't apply by definition, because the anchor Tx locktime is longer than the asset locktime (which has no locktime)

gijswijs avatar Apr 18 '24 08:04 gijswijs

True - actually, I'm not sure which situations an asset locktime would have meaning that wouldn't also be required in the anchor TX.

jharveyb avatar Apr 18 '24 15:04 jharveyb

If we start at the VM level, the following needs to occur:

  • [ ] Add argument to the VM to pass in the block height of the block that includes the transaction.

We don't map the relative_lock_time or lock_time during the mapping of the inputs. Instead we'll map this during the validation process.

During validation of a state transition:

  • For each prev_input within the set of referenced prev_asset_witnesses:
    • [x] Set the sequence number to the relative_lock_time field of the input, if it exists.
    • [x] Set the lock time of the transaction as the lock_time of the input TLV leaf being validated, if it exists.
    • Perform external lock time and relative lock time validation:
      • [ ] If a relative_lock_time field exists, if the input age of the referenced TLV leaf is less than relative_lock_time validation MUST fail.
      • [ ] If a lock_time field exists, if the block height of the block that includes the transaction is less than lock_time validation MUST fail.

gijswijs avatar May 01 '24 13:05 gijswijs

Looking at: https://github.com/lightninglabs/taproot-assets/blob/dc8932dc385aca160586a82e94f4113c717d9992/asset/tx.go#L57-L67

It seems that relative_lock_time and lock_time are already set.

gijswijs avatar May 01 '24 13:05 gijswijs

~~Are we certain that don't already exercise asset locktimes at the vm-level?~~

We use btcsuite's NewEngine with txscript.StandardVerifyFlags which includes ScriptVerifyCheckLockTimeVerify and ScriptVerifyCheckSequenceVerify: https://github.com/lightninglabs/taproot-assets/blob/dc8932dc385aca160586a82e94f4113c717d9992/vm/vm.go#L318-L321

~~A few lines down we execute that engine, which should return locktime errors if there are any?~~ https://github.com/lightninglabs/taproot-assets/blob/dc8932dc385aca160586a82e94f4113c717d9992/vm/vm.go#L325-L328

The reason btcsuite's engine doesn't return with an error is because it only does ScriptVerifyCheckLockTimeVerify or ScriptVerifyCheckSequenceVerify if there are opcodes that give reason to do so. In the case of assets we don't rely on opcodes to verify these values, so it passes regardless of what values are set.

gijswijs avatar May 01 '24 14:05 gijswijs

I think one way to test this would be porting some of the scripts and test cases here:

https://github.com/lightningnetwork/lnd/blob/master/input/script_utils_test.go

To VM test cases. Or as part of tapsend to also cover proof file verification.

Separately, we use CSV in one VM test case template right now:

https://github.com/lightninglabs/taproot-assets/blob/c8978aaaa735b980bbc1ccedee6e0629679da88d/vm/vm_test.go#L218

So AFAICT we need extra checks of the virtual TX input's locktimes against the real blockheight and time, before we run the txscript.Engine in the VM.

jharveyb avatar May 01 '24 18:05 jharveyb

After side meet, Gijs will ship this issue.

dstadulis avatar May 02 '24 19:05 dstadulis