Add Signing module with mkStdTx
#166
Conversation
Anviking
force-pushed
the
anviking/95/mkstdtx
branch
from
05587cc
to
32e9fea
Compare
Anviking
force-pushed
the
anviking/95/mkstdtx
branch
5 times, most recently
from
85b9547
to
3725931
Compare
Anviking
force-pushed
the
anviking/95/mkstdtx
branch
2 times, most recently
from
e82a9b4
to
5025ef3
Compare
| :: Address | ||
| -> (Key 'RootK XPrv, Passphrase "encryption") | ||
| -> s | ||
| -> (Maybe (Key 'AddressK XPrv), s) |
There was a problem hiding this comment.
Here is the AddressScheme class
Anviking
force-pushed
the
anviking/95/mkstdtx
branch
2 times, most recently
from
d6e8806
to
dcf2678
Compare
| -> [(TxIn, TxOut)] | ||
| -- ^ Selected inputs | ||
| -> [TxOut] | ||
| -- ^ Selected outputs (including change) |
There was a problem hiding this comment.
We are missing the change outputs here. Those are part of the CoinSelection but are simply Coins. In practice, we do need to generate change addresses on the internal chain for all change, in sequence.
However, we also want to leave that flexible in the long-run in order to let users pick their own change addresses if needed.
There was a problem hiding this comment.
Ah, assumed they were included in the CoinSelection outputs, but makes a lot of sense
However, we also want to leave that flexible in the long-run in order to let users pick their own change addresses if needed.
Yes, sounds good to me (Edit: with users I thought of callers of mkStdTx)
Adding a function
generateChangeOutput :: s -> Coin -> TxOut
to AddressScheme?
Or have it specific to SeqState?
There was a problem hiding this comment.
It is indeed specific to the scheme state. Your suggestion is sound. We'll see later how we enable this to be set by users 👍
| SignTx (Hash payload) -> "\x01" <> network <> payload | ||
| where | ||
| network = toByteString . CBOR.encodeInt32 $ pm | ||
| pm = 1097911063 -- testnet; TODO: need to decide on how to pass this in |
KtorZ
force-pushed
the
anviking/95/mkstdtx
branch
from
a12ffb5
to
5aaa711
Compare
Not tested yet. For the time being we hard-code the testnet protocol magic in the signing.
KtorZ
force-pushed
the
anviking/95/mkstdtx
branch
from
07d7741
to
42fff2d
Compare
| encodeXPub (publicKey xPrv) <> | ||
| getHash (sign (SignTx tx) (xPrv, pwd)) | ||
| mkWitness tx xPrv = PublicKeyWitness $ | ||
| encodeXPub (publicKey xPrv) <> getHash (sign (SignTx tx) (xPrv, pwd)) |
There was a problem hiding this comment.
There was no particular reason to change that 🤷♂️
| hashTx :: Tx -> Hash "tx" | ||
| hashTx txSigData = Hash | ||
| $ convert | ||
| $ (hash @ByteString @Blake2b_256) | ||
| $ BA.convert |
There was a problem hiding this comment.
Small change here, using qualified import for ambiguous function. The context is probably enough here, but, usually, for functions like this that exist in many modules, it's good to be specific (like map or filter or encode ... )
| emptyPendingIxs :: PendingIxs | ||
| emptyPendingIxs = PendingIxs mempty | ||
|
|
||
| -- | Update the set of pending indexes by discarding every indexes _below_ the |
There was a problem hiding this comment.
@KtorZ Can I suggest calling it like discardIxsBelow then?
There was a problem hiding this comment.
Hmmm.. I wouldn't. So that the logic remains encapsulated in the PendingIxs structure. What the update does is actually irrelevant to the caller so I wouldn't have the name conveying any behavior here but rather, be fairly abstract like "update" or "adjust".
Issue Number
Overview
I'll try and test this firstThink we could get this in firstmkStdTxin a minimal wayshuffleinCoinSelectionAddressSchemeclassComments