Implement cw4-group typescript helper #476
Conversation
There was a problem hiding this comment.
Mostly looks good.
I see two flaws in the API mapping. Could can fix them and merge.
TL;DR we need a new ts-helpers repo somewhere for all this code, and it should be published to npm to make life easy for app developers, not just @cosmjs/cli.
It brings up a number of larger questions for me however:
- Where should this ts code really live (maybe a different repo that is just TS)?
- How to best reuse code? - there is common network config/setup code in all of these.
- How to import this into an app (not just the cli)?
- How to test?
I would strongly propose splitting these code into 2 orthogonal pieces - network setup (useOptions and the eg. pebblenet options) and the contract helpers.
They should be published to npm and easily importable from frontend apps.
For the cli, one could use two setup methods, eg..:
Usage: npx @cosmjs/cli@^0.26 --init https://raw.githubusercontent.com/CosmWasm/cw-cli/master/pebblenet.ts --init https://raw.githubusercontent.com/CosmWasm/cw-plus/master/contracts/cw4-group/helpers.ts
Then the rest of the functions would work as in the docs, just pulling the Network code from a different place than the group code.
| } | ||
| } | ||
|
|
||
| const pebblenetGasPrice = GasPrice.fromString("0.01upebble"); |
There was a problem hiding this comment.
pebble net config is repeated in each helper, right?
There was a problem hiding this comment.
TL;DR we need a new ts-helpers repo somewhere for all this code, and it should be published to npm to make life easy for app developers, not just @cosmjs/cli.
Great we had some plans to move to. https://github.com/InterWasm/DAO/issues/26 @findolor
| recoverMnemonic: (password: string, filename?: string) => Promise<string> | ||
| } | ||
|
|
||
| const useOptions = (options: Options): Network => { |
contracts/cw4-group/helpers.ts
Outdated
| } | ||
|
|
||
| interface MemberListResponse { | ||
| readonly members: number; |
There was a problem hiding this comment.
This should be something like:
interface MemberListResponse {
readonly members: []Member;
}
interface Members {
readonly addr: string;
readonly weight: number;
}See:
#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
pub struct Member {
pub addr: String,
pub weight: u64,
}
#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
pub struct MemberListResponse {
pub members: Vec<Member>,
}
contracts/cw4-group/helpers.ts
Outdated
|
|
||
| // actions | ||
| updateAdmin: (txSigner: string, admin?: string) => Promise<string> | ||
| updateMembers: (txSigner: string, remove: string[], add: string[] ) => Promise<string> |
There was a problem hiding this comment.
See:
UpdateMembers {
remove: Vec<String>,
add: Vec<Member>,
},The add argument is a []Member like above in the query... it must have both the address and the weight
contracts/cw4-group/helpers.ts
Outdated
| // actions | ||
| updateAdmin: (txSigner: string, admin?: string) => Promise<string> | ||
| updateMembers: (txSigner: string, remove: string[], add: string[] ) => Promise<string> | ||
| addHook: (txSigner: string, addr: string) => Promise<string> |
There was a problem hiding this comment.
I highly doubt these will be called by any client code. I would remove them to avoid confusing devs
There was a problem hiding this comment.
Added _ prefix
// will not used by end user for testing purposes
_addHook: (txSigner: string, addr: string) => Promise<string>
_removeHook: (txSigner: string, addr: string) => Promise<string>| }; | ||
| } | ||
|
|
||
| const downloadWasm = async (url: string): Promise<Uint8Array> => { |
There was a problem hiding this comment.
This helper may belong in a standard library or something.
This PR implements ts helper for cw4-group contract.