-
Notifications
You must be signed in to change notification settings - Fork 38
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.
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
refactor(sdk): operatorContractUtils
type safety
#2919
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a strict sense, I don't mind the contract addresses being just plain strings here since EthereumAddress
type comes with the lower casing requirement which is not strictly necessary here. However I'm not against using it here either so I'm fine with that change.
} | ||
|
||
export async function generateWalletWithGasAndTokens(opts?: GenerateWalletWithGasAndTokensOpts): Promise<Wallet & SignerWithProvider> { | ||
const provider = getProvider() | ||
export async function createTestWallet(): Promise<Wallet & SignerWithProvider> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't always want to spend time waiting for a wallet to be generated that contains tokens, therefore I like the old name since it indicates what kind of wallet we are generating. Sometimes in tests we have wallets that don't need to tokens or gas (just generate random private key), need only gas (use KeyServer) or need both (use this function). I think the name should indicate that we are now generating a certain kind of wallet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would make sense to modify dev-chain-fast
so that they KeyServer keys have also some tokens. Then this utility function would be obsolete. https://linear.app/streamr/issue/ETH-800/dev-chain-fast-mint-data-tokens-for-pre-baked-accounts
bigint
type when a number is a wei amountEthereumAddress
type for contract addressesSigner
instead ofWallet
client.getSigner()
as an argumentOther changes
generateWalletWithGasAndTokens()
->createTestWallet()
setupOperatorContract(
) ->setupTestOperatorContract()
deployOperatorContract()
->deployTestOperatorContract()
deploySponsorshipContract()
->deployTestSponsorshipContract()
getProvider()
->getTestProvider()
getTestAdminWallet()
(not used by any component)transferTokens()
fromoperatorContractUtils.ts
(it is an internal helper method)Future improvements
sponsor
/stake
/delegate
etc. methods both for in production environment and in teststoken
parameter fortransferTokens
should be required or injected from the context (now it is implicitly test token)setupTestOperatorContract()
anddeployTestOperatorContract()
deployTestOperatorContract()
could return client'sOperator
instance?