-
Notifications
You must be signed in to change notification settings - Fork 214
SCP-3305 SCP-3263 fixed Ledger.Constraints.Offchain.updateUtxoIndex #275
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but I have not worked in this area recently, so I asked @sjoerdvisscher for a review also.
The reason we removed public-key inputs is that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do this, the way to go would be to revert #57.
Yes, I've tested this will wallet and actual Marlowe contract endpoints. The I stepped through each phase of wallet's processing of the request from the PAB to see how it balances the transaction: the wallet makes provision for spending particular UTxOs in the |
696c043
to
4d0869e
Compare
4d0869e
to
d08145b
Compare
@sjoerdvisscher, I reverted #57 in d08145b. The Marlowe PAB integration test passes (running the full Marlowe contract on our private testnet) using this revision. Would you please re-review? Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like all this red :)
Prior to this commit,
Ledger.Constraints.Offchain.updateUtxoIndex
would discard lookups for inputs that aren't script outputs. However, sometimes it is necessary to include particular public-keyTxIn
s in a transaction, viaPlutus.Contract.Wallet.ExportTx.lookups
: a good example of this is when aPlutus.Contract.Currency.OneShotCurrency
needs to consume a specifiedUTxO
in its minting policy. Even thoughPlutus.Constraints.Tx.mustSpendPubKeyOutput
adds a public-key input to the lookups,updateUtxoIndex
discards that, with the result that it is only by chance thatcardano-wallet
would select that necessary input for the balanced transaction and provide it to Plutus validators. (Note thatcardano-wallet
does not automatically provide theTxIn
in the partially constructed transaction to validators: it only providesCardano.Wallet.PartialTx.inputs
and the inputs it has semi-randomly chosen.) The consequence of all of this is that scripts that require a particular input will randomly fail during balancing in wallets with more than one UTxO.This fix simply adds public-key inputs to the type
Ledger.Constraints.OffChain.ScriptOutput
so that it can also hold public-key inputs. (There might be a more descriptive name thanScriptOutput
now.) It makes minor adjustments to the chain of functions that pass this into an unbalanced transaction before it is sent tocardano-wallet
.Pre-submit checklist: