Skip to content
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

[ADP-3272] WIP: Rough sketch of UTxO handling simplification. #4451

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonathanknowles
Copy link
Member

@jonathanknowles jonathanknowles commented Feb 14, 2024

🏗️ This PR does not compile -- it's just a rough sketch of a potential simplification of balanceTransaction.

TODO:

  • Specify the difference in the specification.
  • PartialTxWithUTxO -- specify what will happen to this.
  • [ ]

Issue

ADP-3272

-- transaction.
-> Set TxIn
-- ^ Defines the subset of UTxOs that may be spent in addition to any
-- UTxOs that are already spent by a transcation.
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
-- UTxOs that are already spent by a transcation.
-- UTxOs that are already spent by a transaction.

@jonathanknowles jonathanknowles force-pushed the jonathanknowles/adp-3272/simplify-utxo-handling branch from 5afc550 to 9052584 Compare February 14, 2024 06:37
@jonathanknowles jonathanknowles changed the title WIP: Prototype simplification of UTxO handling. WIP: Sketch of UTxO handling simplification. Feb 14, 2024
@@ -3588,7 +3593,7 @@ balanceTransaction

pure $ Write.PartialTx
(Write.fromCardanoApiTx tx)
externalUTxO
mempty -- externalUTxO
Copy link
Member Author

Choose a reason for hiding this comment

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

To be deleted ^^^

@@ -2386,10 +2386,10 @@ buildTransactionPure
(Left $ unsafeShelleyOnlyGetRewardXPub @s (getState wallet))
txCtx
(Left preSelection)
let utxoAll = Write.fromWalletUTxO utxo
let utxoSpendable = Map.keysSet (unUTxO utxoAll)
Copy link
Member Author

Choose a reason for hiding this comment

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

In the case where there is no partial transaction available, we allow balanceTx to spend from the entire UTxO set.

@jonathanknowles jonathanknowles changed the title WIP: Sketch of UTxO handling simplification. WIP: Rough sketch of UTxO handling simplification. Feb 14, 2024
@jonathanknowles jonathanknowles changed the title WIP: Rough sketch of UTxO handling simplification. [ADP-3272] WIP: Rough sketch of UTxO handling simplification. Feb 14, 2024
Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

While neat, I believe keeping the "extra utxo" alongside the Tx whose inputs it resolves makes conseptual sense. So I'd be hesitent in going away from this.

For instance, we could imagine writing the following helper function:

addInput :: (TxIn, TxOut era) -> PartialTx era -> PartialTx era
addInput (inp, out) = -- add inp to tx, add (inp, out) to extraUTxO

I agree the wallet-influenced notion of UTxO conflict (different value or address) ideally shouldn't exist in cardano-balance-tx, but perhaps there is another solution to it which may become more apparent in the future. I don't think it's a problem worth thinking too much about at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants