Support for addresses with staking keys #103 [obsolete]. #134
Conversation
|
I haven't added any tests yet. All the tests are left unchanged, that is they are initialized with enterprise addresses (no stake). Some of them rely on running with enterprise address. |
|
TODO: Update changelog, but once we're settled with interface change |
| data TestWallet = TestWallet | ||
| { twInitDistribiution :: [Positive] | ||
| , twExpected :: Maybe (ValueOrdering, Value) | ||
| , hasStakeKeys :: Bool |
There was a problem hiding this comment.
Maybe make separate type for key, like KeyType = Enterprise | WithStakeing to avoid boolean boolean blindness.
There was a problem hiding this comment.
KeyType
probably, AddressType will be better
| [shouldFail] | ||
| , -- Tests with wallet's Value assertions | ||
| assertExecution | ||
| "Pay from wallet to wallet" | ||
| (initAda [100] <> initAndAssertAda [100, 13] 123) | ||
| (withContract $ \[pkh1] -> payTo pkh1 10_000_000) | ||
| (withContract . onEnterpriseWallets $ \[pkh1] -> payTo pkh1 10_000_000) |
There was a problem hiding this comment.
Hmm, I'm not sure if onEnterpriseWallets needed. Maybe just give user
withContract $ \walletInfos :: [WalletInfo] -> ...as it will be breaking API change anyway?
There was a problem hiding this comment.
And probably change payTo contracts to accept address as 1st argument.
There was a problem hiding this comment.
i was thinking of it only as a helper to quickly adjust existing tests with little effort and not something that should be used in newly written tests. I would leave that in the exports, though as our tests double as examples for documentation, i see that i should actually go through them and update to use new api.
| TestWallet (..), | ||
| compareValuesWith, | ||
| ValueOrdering (..), | ||
| WalletInfo (..), |
There was a problem hiding this comment.
We should be careful with exporting constructors, I think.
I see there is smart constructor makeWalletInfo which builds ownAddress depending on staking keys presence or absence. If this is invariant of WalletInfo, then exposing WalletInfo constructor can allow creation of invalid instates.
(or maybe make ownAddress function that computes address when it's called, not the filed of ADT)
There was a problem hiding this comment.
right, I would hide the constructor
| Contract w s e a -> | ||
| m () | ||
| runContract_ e w c = void $ runContract e w c | ||
|
|
||
| runContract :: | ||
| (ToJSON w, Monoid w, MonadIO m) => | ||
| ClusterEnv -> | ||
| BpiWallet -> | ||
| WalletInfo -> |
There was a problem hiding this comment.
zmrocze
force-pushed
the
karol/staking-keys
branch
from
be4f410 to
a63d2bb
Compare
|
I reverted this pr to the last green commit and pr'ed latest changes instead to #136. This is because my intention turned out to be impossible to implement ;/ more in that pr. We need to go with some simpler aproaches |
zmrocze
changed the title
|
Subsumed by #136 |
addWalletswithStakeKeysto add stake keys toTestWalletspcOwnStakePubKeyHashfor BPI.WalletInfo's to contracts instead ofPubKeyHash's.onEnterpriseAddresseshelper.withContract.