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

SCP-3130: Tx constraint to specify stake stake key hash with payment #181

Merged
merged 1 commit into from
Dec 10, 2021

Conversation

koslambrou
Copy link
Contributor

@koslambrou koslambrou commented Dec 9, 2021

Modified the tx creation constraints (plutus-ledger-constraints repo) to allow to specify the stake public key hash of a transaction output address.

This isn't strictly necessary, since you can make payments using only the payment key. However, it is useful when you want to derive a specific Cardano address (bech32 format) which needs the payment key + stake key.

While doing that, I also took the time to add a additionnal newtype wrappers to PubKeyHash such as PaymentPubKeyHash, PaymentPubKey, StakePubKeyHash and StakePubKey in order to clearly understand to which PubKeyHash we are refering to. This implied a lot of code change. These datatypes where defined in Ledger.Address.

Related to that, I changed the name of WalletEffect.ownPubKeyHash to WalletEffect.ownPaymentPubKeyHash. Maybe it also makes sense to also add the effect WalletEffect.ownStakePubKeyHash, but this should be in a separate PR.

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Comment on lines +67 to +75
{-# INLINABLE pubKeyHashAddress #-}
-- | The address that should be targeted by a transaction output locked by the
-- given public payment key (with it's public stake key).
--
-- TODO: This should be moved to Plutus.V1(or V2).Ledger.Address with the newtypes.
pubKeyHashAddress :: PaymentPubKeyHash -> Maybe StakePubKeyHash -> Address
pubKeyHashAddress (PaymentPubKeyHash pkh) skh =
Address (PubKeyCredential pkh)
(fmap (StakingHash . PubKeyCredential . unStakePubKeyHash) skh)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michaelpj This was copied from plutus because it's own pubKeyHashAddress would set the stake credential to Nothing. Is it okay if modify that function on the plutus side? V1 or V2?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, fine to modify it, it's not used by the key bit.

@koslambrou koslambrou force-pushed the scp-3130-mustPayToAddress branch 5 times, most recently from 4dca821 to 885c6df Compare December 9, 2021 16:43
…e stake public key hash of a transaction output address.

* Added the constraints `mustPayToPubKeyAddress` and `mustPayWithDatumToPubKeyAddress`

* Added newtype wrappers to differentiate between public key hashes: `PaymentPubKeyHash`, `PaymentPubKey`, `StakePubKeyHash` and `StakePubKey`.

* Added newtype wrapper 'PaymentPrivateKey'

* Replaced most `PubKeyHash` and `PubKey` in code with the newly defined newtypes.

* Modified the name of `WalletEffect.ownPubKeyHash` to `WalletEffect.ownPaymentPubKeyHash`.

* Added the prefix 'mock' to the name of functions in `Wallet.Emulator.Wallet` to refer to Mock wallets.

* Formatted the modified modules based on the `-Wmissing-import-lists` GHC flag.
@koslambrou koslambrou force-pushed the scp-3130-mustPayToAddress branch from 885c6df to 51ac83f Compare December 9, 2021 18:36
Copy link
Contributor

@silky silky left a comment

Choose a reason for hiding this comment

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

Looks good! I think it's a net win for readability as well :)

@koslambrou koslambrou merged commit 46e831e into main Dec 10, 2021
@koslambrou koslambrou deleted the scp-3130-mustPayToAddress branch December 10, 2021 15:46
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.

3 participants