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 3 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.