[ADP 3491] Relax policy key guard

[paweljakubas]

policyXPub is needed in constructTransaction in two cases:

• tx contains minting/burning action
• tx deals with reference scripting and has reference script template nonempty

instead of using policyXPub we use policyXPubM throught the function and only deconstruct it when checking those two cases. ErrReadPolicyPublicKeyAbsen is used for the those two valid situations when policy xpub is missing.

Comments

Issue Number

Fix #4872

[Anviking]

🤔 Having to resort to an approach with fromJust seems unfortunate...

[paweljakubas]

yeah, fair point. Now no partial function fixing ;-)

[paweljakubas]

waiting for the daedalus team to confirm if the fix works. If yes then we will undraft it.

[abailly]

There's a remaining issue: constructTransaction depends on decodeTransaction which unconditionally extract the policy pub key and throws an exception if it cannot find one.

[abailly]

Fix #4872

[Anviking]

@abailly the last commit is huge (+3,919 −3,109). Unintentionally caused by some autoformatter?

[abailly]

@Anviking I don't know, I ran the code checker and it did not complain 🤷

[abailly]

I must confess I pay exactly 0 attention to code format as I expect this to be handled by the tooling. In our case, I understand this is stylish-haskell, which I ran.

[abailly]

@paweljakubas I think I would like to write a proper test for this, your guidance would be much appreciated.

[paweljakubas]

The most beautiful (and persuasive also) test (integration one) would be the following:

1. Create wallet with funds, walMnemonic
2. Get extended account public key from walMnemonic, xAcctPub
3. Create account public key wallet from xAcctPub, walAcct
4. Create constructTransation for walAcct that joinDep in conway only (no babbage)
5. Hand over cbor of point 4 to walMnemonic and sign and submit it
6. Make sure both walAcct and walMnemonic are voting.

In essense, walMnemonic can be treated as hardware wallet.

[abailly]

@Anviking I updated the PR, the size of the diff is much more reasonable :)
I really wish we move to a formatter that actually enforces proper formatting, eg. ormolu or fourmolu, in order to streamline this whole thing

[Anviking]

lgtm, if you've tested this. I suspect the fromJust pattern could be dropped but that already existed before this pr.

[abailly]

@Anviking I agree an integration test would be useful, but I also don't want to add too many of those as they might become a liability pretty quickly.

[Anviking]

Yeah, I just meant that I presume you've verified (manually) that this did what you wanted it to

[abailly]

Of course, but you're right and this feels a bit sloppy

[Anviking]

but I also don't want to add too many of those as they might become a liability pretty quickly.

Fully share this concern though