[ADP-3372] Deposit Wallet - json roundtrips for types #4710
Conversation
| , Customer | ||
| , fromRawCustomer | ||
|
|
There was a problem hiding this comment.
| , Customer | |
| , fromRawCustomer |
Wait — the concern of the Read modules is to read data as specified in the ledger specification, and as it appears on the blockchain, but Customer is not part of that. 😅 (The name Deposit is accidental, I want to merge this module with Cardano.Wallet.Read but had to choose a different name here in order to avoid a name clash.)
Could you leave the Customer type where it is, and instead solve the Word32 → Customer conversion directly in the Cardano.Wallet.Deposit.HTTP.JSON.JSONSpec module? 🤓
There was a problem hiding this comment.
got it - reverted the commit
|
|
||
| prop_jsonRoundtrip :: (Eq a, Show a, FromJSON a, ToJSON a) => a -> Property | ||
| prop_jsonRoundtrip val = | ||
| decode (encode val) === Just val |
|
|
||
| instance Arbitrary (ApiT Customer) where | ||
| arbitrary = | ||
| ApiT . fromRawCustomer <$> arbitrary |
There was a problem hiding this comment.
| ApiT . fromRawCustomer <$> arbitrary | |
| ApiT <$> chooseBoundedIntegral |
🤓 On the matter of the Word32 → Customer conversion: Somewhat unusually, I have chose to make Customer a concrete type rather than an abstract data type — specifically, I have defined Customer = Word31. In other words, the deposit wallet will only support customer IDs in the range 0 … 2^31-1!
This is intentional, and due to the BIP-32 address derivation, where the index for one derivation step is exactly a Word31. I could have used longer indices, but I decided to keep this limit for compatibility with hardware wallets; I don't want to explain derivations with arbitrary length. Hence, Customer = Word31 is baked into the Deposit Wallet.
There was a problem hiding this comment.
introduced fromRawCustomer that creates Customer from Word31 and added Arbitrary Word31.
| listLen <- chooseInt (0, 100) | ||
| let genPair = (,) <$> (unApiT <$> arbitrary) <*> (unApiT <$> arbitrary) | ||
| ApiT <$> vectorOf listLen genPair |
There was a problem hiding this comment.
| listLen <- chooseInt (0, 100) | |
| let genPair = (,) <$> (unApiT <$> arbitrary) <*> (unApiT <$> arbitrary) | |
| ApiT <$> vectorOf listLen genPair | |
| let genPair = (,) <$> (unApiT <$> arbitrary) <*> (unApiT <$> arbitrary) | |
| ApiT <$> listOf genPair |
Perhaps? listOf depends on the QuickCheck size parameter, which usually composes better. But since we don't care about composability here, vectorOf is also fine. I leave this up to you.
paweljakubas
force-pushed
the
paweljakubas/adp-3372/json-roundtrips
branch
from
bffed90 to
d72ceda
Compare
paweljakubas
force-pushed
the
paweljakubas/adp-3372/json-roundtrips
branch
from
d72ceda to
959531b
Compare
| @@ -135,6 +136,9 @@ isCustomerAddress :: Address -> WalletState -> Bool | |||
| isCustomerAddress address = | |||
| flip Address.isCustomerAddress (Read.toRawAddress address) . addresses | |||
|
|
|||
| fromRawCustomer :: Word31 -> Customer | |||
| fromRawCustomer = fromIntegral | |||
There was a problem hiding this comment.
| fromRawCustomer = fromIntegral | |
| fromRawCustomer = id |
As much as I love abstract data types — this one is positively a type synonym. 🤓
Customertype relocationApiT AddressApiT CustomerApiT CustomerListComments
Issue Number
adp-3372