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

Support extraCollateral in updateSealedTx #2941

Merged
merged 2 commits into from
Oct 1, 2021

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Oct 1, 2021

  • Support extraCollateral in updateSealedTx
  • Extend property test
  • Fix missing conversion in fromAlonzoTxBodyAndAux

Comments

Issue Number

ADP-1140

Discovered in the property test of the following commit. The
resolvedCollateral would always be empty.
Will be needed for the balancing endpoint.
@Anviking Anviking self-assigned this Oct 1, 2021
@@ -1206,8 +1206,7 @@ fromAlonzoTxBodyAndAux bod mad =
, fee =
Just $ fromShelleyCoin fee
, resolvedCollateral =
-- TODO: (ADP-957)
[]
map ((,W.Coin 0) . fromShelleyTxIn) (toList collateral)
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess these W.Coin 0 are remainders from jormungandr? 🤷‍♂️

@@ -435,6 +435,9 @@ mkDelegationCertificates da accXPub =
-- `updateSealedTx`. See `updateSealedTx` for more details.
data ExtraTxBodyContent = ExtraTxBodyContent
{ extraInputs :: [TxIn]
, extraCollateral :: [TxIn]
-- ^ Only used in the Alonzo era and later. Will be silently ignored in
-- previous eras.
Copy link
Member Author

Choose a reason for hiding this comment

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

Thought about erroring, but decided to ignore instead.

I don't think returning Left here is a good idea, as passing collateral to a Shelley tx would be a programmer and not user error.

@Anviking Anviking requested a review from paweljakubas October 1, 2021 10:11
ShelleyBasedEraAlonzo -> body
{ Alonzo.outputs = Alonzo.outputs body
<> StrictSeq.fromList (Cardano.toShelleyTxOut era <$> extraOutputs')
, Alonzo.inputs = Alonzo.inputs body
<> Set.fromList (Cardano.toShelleyTxIn <$> extraInputs')
, Alonzo.collateral = Alonzo.collateral body
<> Set.fromList (Cardano.toShelleyTxIn <$> extraCollateral')
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

outs = Set.fromList . view #outputs . _decodeSealedTx
fee = view #fee . _decodeSealedTx

isAlonzo (cardanoTx -> InAnyCardanoEra Cardano.AlonzoEra _) = True
isAlonzo (cardanoTx -> InAnyCardanoEra _ _) = False
Copy link
Contributor

Choose a reason for hiding this comment

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

I like ViewPatterns more and more:-)

Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

lgtm!

@Anviking
Copy link
Member Author

Anviking commented Oct 1, 2021

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 1, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit eb997a7 into master Oct 1, 2021
@iohk-bors iohk-bors bot deleted the anviking/ADP-1140/extraCollateral branch October 1, 2021 11:42
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