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.

Sign up for GitHub

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

Jump to bottom

Add initial support for serializing signed transaction #362

Merged
merged 2 commits into from
Jun 14, 2019
+57 −8
Merged

Add initial support for serializing signed transaction #362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15c13db
Add initial support for serializing signed transaction
akegalj Jun 4, 2019
ff9e902
Fix review comments
akegalj Jun 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions lib/jormungandr/src/Cardano/Wallet/Jormungandr/Api.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import Prelude
import Cardano.Wallet.Jormungandr.Binary
( FromBinary (..), runGet )
import Cardano.Wallet.Primitive.Types
( Block, Hash (..) )
( Block, Hash (..), Tx, TxWitness )
import Data.Binary.Get
( getByteString )
import Data.ByteArray.Encoding
Expand Down Expand Up @@ -91,13 +91,10 @@ type GetTipId

type PostSignedTx
= "v0"
:> "transaction"
:> ReqBody '[JormungandrBinary] SignedTx
:> "message"
Anviking marked this conversation as resolved.
Show resolved Hide resolved
:> ReqBody '[JormungandrBinary] (Tx, [TxWitness])
:> Post '[NoContent] NoContent

-- TODO: Replace SignedTx with something real
data SignedTx

newtype BlockId = BlockId (Hash "BlockHeader")
deriving (Eq, Show)

Expand Down
56 changes: 54 additions & 2 deletions lib/jormungandr/src/Cardano/Wallet/Jormungandr/Binary.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeApplications #-}

-- |
-- Copyright: © 2018-2019 IOHK
Expand All @@ -19,6 +21,8 @@ module Cardano.Wallet.Jormungandr.Binary
, getBlockHeader
, getBlock

, putTransaction

, ConfigParam (..)
, ConsensusVersion (..)
, LeaderId (..)
Expand All @@ -40,7 +44,7 @@ module Cardano.Wallet.Jormungandr.Binary
import Prelude

import Cardano.Wallet.Jormungandr.Environment
( Network (..) )
( KnownNetwork (..), Network (..) )
import Cardano.Wallet.Primitive.Types
( Address (..)
, Coin (..)
Expand All @@ -67,10 +71,14 @@ import Data.Binary.Get
, runGet
, skip
)
import Data.Binary.Put
( Put, putByteString, putWord64be, putWord8 )
import Data.Bits
( shift, (.&.) )
import Data.ByteString
( ByteString )
import Data.Proxy
( Proxy (..) )
import Data.Quantity
( Quantity (..) )
import Data.Word
Expand Down Expand Up @@ -216,10 +224,54 @@ getTransaction = label "getTransaction" $ isolate 43 $ do
error "unimplemented: Account witness"
other -> fail $ "Invalid witness type: " ++ show other


putTransaction :: forall n. KnownNetwork n => Proxy n -> (Tx, [TxWitness]) -> Put
putTransaction _ (tx, witnesses) = do
putTokenTransfer (Proxy @n) tx
mapM_ putWitness witnesses

putWitness :: TxWitness -> Put
putWitness witness =
case witness of
PublicKeyWitness xPub (Hash sig) -> do
-- Witness sum type:
-- * 1 for old address witnness scheme
-- * 2 for new address witness scheme
-- * 3 for account witness
putWord8 1
Anviking marked this conversation as resolved.
Show resolved Hide resolved
putByteString xPub
putByteString sig
-- TODO: note that we are missing new address type witness:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we still didn't cover acount and new address witness scheme

Copy link
Member

Choose a reason for hiding this comment

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

Why though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KtorZ
@Anviking and I thoght that it would be ok to start with what we are confident with and extend other bits later

Copy link
Member

Choose a reason for hiding this comment

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

Got you, though I am bit sad to see that we only merge this while the whole story was expected to be completed already 8 days ago.

-- * https://github.com/input-output-hk/rust-cardano/blob/3524cfe138a10773caa6f0effacf69e792f915df/chain-impl-mockchain/doc/format.md#witnesses
-- * https://github.com/input-output-hk/rust-cardano/blob/e0616f13bebd6b908320bddb1c1502dea0d3305a/chain-impl-mockchain/src/transaction/witness.rs#L23
ScriptWitness _ -> error "unimplemented: serialize script witness"
RedeemWitness _ -> error "unimplemented: serialize redeem witness"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure atm how rust node covers these two cases. Maybe they are encoded under old or normal utxo witness scheme



{-------------------------------------------------------------------------------
Common Structure
-------------------------------------------------------------------------------}

putTokenTransfer :: forall n. KnownNetwork n => Proxy n -> Tx -> Put
putTokenTransfer _ (Tx inputs outputs) = do
putWord8 $ fromIntegral $ length inputs
putWord8 $ fromIntegral $ length outputs
mapM_ putInput inputs
mapM_ putOutput outputs
where
putInput (TxIn inputId inputIx) = do
-- NOTE: special value 0xff indicates account spending
-- only old utxo/address scheme supported for now
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to recall what is new address scheme. I am not sure how to correctly map what we said here https://github.com/input-output-hk/cardano-wallet/blob/master/lib/core/src/Cardano/Wallet/Primitive/Types.hs#L481 with jow rust node encode operates https://github.com/input-output-hk/rust-cardano/blob/3524cfe138a10773caa6f0effacf69e792f915df/chain-impl-mockchain/doc/format.md#token-transfer

Address (bootstrap address 33 bytes, delegation address 65 bytes, account address 33 bytes)

putWord8 $ fromIntegral inputIx
putByteString $ getHash inputId
putOutput (TxOut address coin) = do
putAddress address
putWord64be $ getCoin coin
putAddress address = do
-- NOTE: only single address supported for now
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that we actually have support for other kinds of addresses (old/new/grouped) https://github.com/input-output-hk/cardano-wallet/blob/master/lib/core/src/Cardano/Wallet/Primitive/Types.hs#L481 - so we should be able to implement this I think

putWord8 (single @n)
putByteString $ getAddress address

getTokenTransfer :: Get ([TxIn], [TxOut])
getTokenTransfer = label "getTokenTransfer" $ do
inCount <- fromIntegral <$> getWord8
Expand All @@ -232,7 +284,6 @@ getTokenTransfer = label "getTokenTransfer" $ do
-- NOTE: special value 0xff indicates account spending
index <- fromIntegral <$> getWord8
tx <- Hash <$> getByteString 32

return $ TxIn tx index

getOutput = do
Expand Down Expand Up @@ -260,6 +311,7 @@ getTokenTransfer = label "getTokenTransfer" $ do
0 -> Mainnet
_ -> Testnet


{-------------------------------------------------------------------------------
Config Parameters
-------------------------------------------------------------------------------}
Expand Down