-
Notifications
You must be signed in to change notification settings - Fork 360
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
Upgrade to Launchpad #311
Upgrade to Launchpad #311
Conversation
Now that CosmWasm/wasmd#222 is solved, is this blocked on any external requirement? I guess just me tagging a stable |
Perfect. I just need that tag on dockerhub. Then we should have everything to continue. |
|
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.
Looks good to me. A few minor comments.
Looking forward to getting this in
@@ -335,7 +335,7 @@ describe("wasm", () => { | |||
// upload data | |||
const hackatom = getHackatom(); | |||
const result = await uploadContract(client, wallet, hackatom); | |||
expect(result.code).toBeFalsy(); | |||
assert(!result.code); |
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.
why is this better?
we are checking for 0
or undefined
if I understand
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.
Because assert
throws if the condition is falsy, expect
reports an error and continues. But in case the transaction failed, all the following checks are pointless and just hide the problem.
@@ -430,6 +422,49 @@ describe("wasm", () => { | |||
expect(await client.wasm.getContractInfo(nonExistentAddress)).toBeNull(); | |||
}); | |||
|
|||
it("can list contract history", async () => { |
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.
nice
}; | ||
} | ||
|
||
function setupWasmExtension(base: LcdClient): WasmExtension { | ||
function setupBankExtension(base: LcdClient): BankExtension { |
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.
Why is this in the test code?
Don't we have such an implementation in the non-test (production) code?
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 have, but we need to reduce dependencies. Otherwise lcdclient depends on bank extension and bank extension depends on lcdclient. Having a wasm dependency here before was a huge upgrade pain.
}; | ||
|
||
async function main() { | ||
const wallet = await Secp256k1Wallet.fromMnemonic(faucet.mnemonic); |
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.
good minimal example of how to use the wallet in tests
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 link this (or other such example) from a README
@@ -0,0 +1 @@ | |||
eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJjcmVhdGVkIjoiMjAyMC0wNy0yMyAxNTowNTo0OC4yMDIyMjEgKzAwMDAgVVRDIG09KzAuNDA3MTQ2NDAxIiwiZW5jIjoiQTI1NkdDTSIsInAyYyI6ODE5MiwicDJzIjoiZXhEVEM1dGVraTVyMDg4ZyJ9.AluhnvIPJAJZZIUwCATLZ2eQNBy-bqUMOwa1QVokRPU4MWOsuxx-TQ.3vXpP5cMQEXdSBko.l5BzPkI5_DUm-iUBM9T9po-9YXYmGK62eieeFVA_WzESZPYxk51fQq_8DCdhh6Y6DhmmvQXPv3hjqQgIgdxPRBFlJwNawrqLElWIVuiS02tAjwQXAw6to0sdNtJc5r8Fzr82aD_lRMDiJ5ZoJ6i3wHLgmfwtOH0JdGwGFwiEQYuWX0SK0Ms2LHUydmsriX6MbHik24R061Qc3dR0GfclCb9dP_jRvXBhen8EJyt3cM3opYZ5lqDcLv_OQnDaFb4.Ctl177TJVkOiSYSaG3IBBg |
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.
I think there is a typo here 😛
just kidding... had to saw something when scrolling over tons of auto-gen code
@@ -8,6 +8,17 @@ | |||
# specified in this config (e.g. 0.25token1;0.0001token2). | |||
minimum-gas-prices = "" | |||
|
|||
# default: the last 100 states are kept in addition to every 500th state; pruning at 10 block intervals |
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.
👍
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.
LGTM
@@ -29,6 +30,9 @@ describe("StakingExtension", () => { | |||
amount: coins(25000, "ucosm"), | |||
gas: "1500000", // 1.5 million | |||
}; | |||
const consesnsusPubkey = |
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.
consensusPubkey
Based on #314Closes #306
Dependencies