Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CIP-0131? | Transaction swaps #880
base: master
Are you sure you want to change the base?
CIP-0131? | Transaction swaps #880
Changes from 5 commits
5f370fd
78c2df2
4fb4ba9
e7a3bdc
85cb791
be3af12
39991dd
ca4b8f6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think this still needs something like the original
script_data_hash
to guarantee the properplutus_data
is used as the redeemer for each smart contract. Otherwise, there is a possible man-in-the-middle attack where a malicious party can change the supplied redeemer to one that benefits them. Wouldn't the following work?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.
The transaction swap is signed, so no-one can mess with the datum that will be passed to the swap scripts. Transaction builder will not have the capability to supply plutus datum's, it will only be the execution units that are supplied:
https://github.com/lehins/CIPs/blob/4fb4ba9d2b4a1164742607036aaa5e8566315819/CIP-xxxx/README.md?plain=1#L119-L130
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.
Datums and redeemers are separate things. Datums are part of the UTxO set so the input itself has everything needed to verify the proper datum is being used. The same is not true for redeemers. The above
swap_body
does not contain theplutus_data
(or hash of it) for the redeemers anywhere. AFAIU that was the point of thescript_data_hash
being in thetransaction_body
.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.
Datums was a terrible terminology. The way we use it internally in Ledger (not sure of that is the same outside) is there are spending datums and datums that are supplied in the redeemers. Both are just arguments that are supplied to plutus scripts through plutus context.
You are right. We'll need to move
swap_redeemers
intoswap_body
, so it can be signed. That is because there is no scriptIntegrityHash in the swap body.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.
It seems everyone uses the terms differently. The redeemer for a smart contract developer doesn't contain execution units, it is just the plutus data. The fact that the same term is used with a different definition by the cddl is really confusing... The terms should be standardized.
I don't think this is enough. If we have a list of signed redeemers, how do we know they are being paired up with the right scripts? I think we really need a script integrity check for the transaction swaps; this version just wouldn't care about the execution units. The
swap_redeemers
can remain outside theswap_body
while theswap_script_data_hash
encapsulates the required script+datum+redeemer pairing.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.
The
swap_redeemer_tag
is also relevant for the integrity check since some scripts can be executed as spending, minting, or withdrawals. How do we know the script is being executed as the right type without the script integrity check?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.
Ledger will take of this part in the same way we take care of it right now.
The point of script integrity hash is that it takes pieces outside of the transaction body and through a cryptographic hash it places it into the body, so it can be signed. In other words it is like placing redeemers, datums and current costmodels directly into the transaction body.
So, if we place redeemers (without execution units) into the swap body, then the user by signing the body prevents anyone messing with the arguments of all of the plutus scripts in the swap. Same applies to the purpose (redeemer tag), since that is the key in the redeemer map.
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.
We could of course employ the same process and include script integrity hash in the swap transactions. Maybe that would be even better for consistency with regular transactions, albeit at the cost of slightly higher complexity
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.
No please, calculating and working with script integrity hash is the worst part of offchain library development on Cardano. Frankly redeemers should have just been directly in the body originally.
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 think the way the plutus context would work with this CIP needs to be explicitly stated in the specification section. I'm imagining the
TxInfo
would have a new field:All of the information related to the transaction swaps would only appear in their respective
SwapTxInfo
(eg, the signatures for a swap transaction are only found in that swap transaction'sSwapTxInfo
).Smart contracts also need a quick way to know which context is theirs. The
ScriptPurpose
would need to specify whichSwapTxInfo
this execution is for in addition to the rest of the information in the purpose. An index into the[SwapTxInfo]
would likely be fine.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.
With the new update to Validation Zones (implicit) , this dependency only exists for transactions that specify
requiredTxs
. That is, transactions that are performing exchanges without caring who the counterparty is are now not required to provide collateral for any other transactions, only themselves.For
requiredTxs
, any DAG of dependencies can be formed. As soon as a failing script is encountered, only the failing transaction + the ones it depends on is entered into the block/zone. For this reason, each transaction is phase-1 checked to cover the size-based fee of transactions it depends on so that they can be included in the zone, and shown to Plutus scripts when needed. Note that this is actually not the collateral for running scripts, but only the size-based fee portion. This is cheaper than having to supply collateral for all scripts.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.
After some consideration, it became clear to me that in Validation Zones implicit, transactions do not need to provide collateral for preceding ones (except in
requiredTx
cases discussed above). So, everyone provides their own collateral and it still functions for its intended purpose - to prevent DDoS attacks on nodes by forcing them to run arbitrary amounts of failing scripts.In VZ design, is possible to allow for the special cases where some nodes would be willing to cover collateral of other users by signing users transactions that include the use of their collateral UTxOs, taking on the risk. This should be the exception and not the rule, however.
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 guess my overall comparison would be something like :
VZ and Swaps do :
VZ negatives :
VZ positives :