Skip to content
This repository was archived by the owner on Dec 2, 2024. It is now read-only.

GTH-692: Don't add collateral when not needed#780

Merged
sjoerdvisscher merged 1 commit intomainfrom
GTH-692-limit-collateral-use
Oct 26, 2022
Merged

GTH-692: Don't add collateral when not needed#780
sjoerdvisscher merged 1 commit intomainfrom
GTH-692-limit-collateral-use

Conversation

@sjoerdvisscher
Copy link
Contributor

@sjoerdvisscher sjoerdvisscher commented Oct 24, 2022

Fixes #692

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Formatting, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reference the ADR in the PR and reference the PR in the ADR (if revelant)
    • Reviewer requested

total collateral: Value (Map [(,Map [("",276566)])])
mint: Value (Map [])
fee: Value (Map [(,Map [("",184377)])])
fee: Value (Map [(,Map [("",176237)])])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sjoerdvisscher sjoerdvisscher force-pushed the GTH-692-limit-collateral-use branch 2 times, most recently from bd22d96 to e6f7385 Compare October 26, 2022 12:25
tx1 = Constraints.mustPayToTheScriptWithDatumInTx onChainTxOutRefs
$ Ada.lovelaceValueOf baseLovelaceLockedByScript
tx1 = Constraints.mustPayToTheScriptWithDatumInTx onChainTxOutRefs (Ada.lovelaceValueOf baseLovelaceLockedByScript)
<> foldMap Constraints.mustBeSignedBy keys
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this now needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! Or rather, I wonder why it wasn't needed before. Without it the fee was estimated too low, since it didn't know there was going to be a second signature.

The only explanation I can think of is that the fee estimation was higher because of the collateral inputs, but the actual fee turned out lower because the collateral inputs were not actually used, leaving enough room to cover for the extra signature?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's possible. Not to worry. Thanks!

Copy link
Contributor

@catch-21 catch-21 Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, something to note is that mustBeSignedBy includes required signer in the txbody, which is redundant in the case where onchain validation for this witness is not performed.

It would be good to have a means of telling the balancer of the extra witness without bloating txbody. Worth a ticket?

@sjoerdvisscher sjoerdvisscher force-pushed the GTH-692-limit-collateral-use branch from e6f7385 to a9d0232 Compare October 26, 2022 15:34
@sjoerdvisscher sjoerdvisscher enabled auto-merge (squash) October 26, 2022 17:30
@sjoerdvisscher sjoerdvisscher merged commit 9685151 into main Oct 26, 2022
@sjoerdvisscher sjoerdvisscher deleted the GTH-692-limit-collateral-use branch October 26, 2022 18:51
kayvank pushed a commit that referenced this pull request Oct 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Contract always includes collateral input

3 participants