Plutus V4 ledger api types as per #7342#7846
Conversation
lehins
left a comment
lehins
left a comment
There was a problem hiding this comment.
Looks great! Thank you!
Just a few last moment changes that I thought of during the review.
| data AccountBalanceInterval | ||
| = AccountBalanceLowerBound V2.Lovelace | ||
| | AccountBalanceUpperBound V2.Lovelace | ||
| | AccountBalanceBothBounds V2.Lovelace V2.Lovelace |
There was a problem hiding this comment.
Let's add one more case, which will be a nice optimization, so instead of specifying AccountBalanceBothBounds x (x+1), one can simply say AccountBalanceExact x. This seem s like can be a common case and it will be better for Tx serialization size and probably for DApp execution too.
We already have it in the works on the ledger side: IntersectMBO/cardano-ledger#5922
| | AccountBalanceBothBounds V2.Lovelace V2.Lovelace | |
| | AccountBalanceBothBounds V2.Lovelace V2.Lovelace | |
| | AccountBalanceExact V2.Lovelace |
|
|
||
| data TopTxInfo = TopTxInfo | ||
| { topTxInfoSubTransactions :: [TxInfo] | ||
| , topTxInfoDatums :: Map V3.TxId V2.Datum |
There was a problem hiding this comment.
Question about this field.
Considering that topTxInfoSubTransactions will contain the TxId, getting it would be as simple as:
getTxId txIx = txInfoId (topTxInfoSubTransactions txInfos !! txIx)Finding an index is also possible, but arguably is more expensive:
getTxIx txId = fromJust $ txInfoSubTxIx =<< find ((== txId) . txInfoId) (topTxInfoSubTransactions txInfos)So, the question is, would it be better to supply the index of sub transaction instead of TxId in this Map?
| , topTxInfoDatums :: Map V3.TxId V2.Datum | |
| , topTxInfoDatums :: Map Haskell.Integer V2.Datum |
| , txInfoWithdrawals :: Map V2.Credential V2.Lovelace | ||
| , txInfoDirectDeposits :: Map V2.Credential V2.Lovelace |
There was a problem hiding this comment.
Let's change these to AccountId
| , txInfoWithdrawals :: Map V2.Credential V2.Lovelace | |
| , txInfoDirectDeposits :: Map V2.Credential V2.Lovelace | |
| , txInfoWithdrawals :: Map AccountId V2.Lovelace | |
| , txInfoDirectDeposits :: Map AccountId V2.Lovelace |
| , ttisWithdrawals :: Map V2.Credential V2.Lovelace | ||
| , ttisDirectDeposits :: Map V2.Credential V2.Lovelace |
There was a problem hiding this comment.
Ditto
| , ttisWithdrawals :: Map V2.Credential V2.Lovelace | |
| , ttisDirectDeposits :: Map V2.Credential V2.Lovelace | |
| , ttisWithdrawals :: Map AccountId V2.Lovelace | |
| , ttisDirectDeposits :: Map AccountId V2.Lovelace |
| , ttisGuards :: [V2.Credential] | ||
| , ttisRequiredTopLevelGuards :: Map V2.Credential () |
There was a problem hiding this comment.
Actually, now looking at this, makes me realize there is no point in having ttisRequiredTopLevelGuards, since they all will be also present in the ttisGuards field. So, instead we'll turn ttisGuards into a Set and remove ttisRequiredTopLevelGuards
| , ttisGuards :: [V2.Credential] | |
| , ttisRequiredTopLevelGuards :: Map V2.Credential () | |
| , ttisGuards :: Map V2.Credential () |
| findContinuingOutputs _ = PlutusTx.traceError "Le" | ||
| {-# INLINEABLE findContinuingOutputs #-} | ||
|
|
||
| getContinuingOutputs :: ScriptContext -> [V2.TxOut] |
There was a problem hiding this comment.
It is important to point out that "ContinuingOutputs" functions only look into individual sub-transaction/top level transaction, not at the whole transaction, which is technically all that they can do for the "Spending" purpose
zliu41
left a comment
zliu41
left a comment
There was a problem hiding this comment.
LGTM once Alexey's comments are incorporated.
3 participants
Adds Plutus Ledger Api types for PlutusV4 as per #7342
ledger API types are not fully finalized yet