UI & Backend improvements to the Mesh Security testing UI by josefleventon · Pull Request #2 · CosmWasm/mesh-security-ui

josefleventon · 2022-10-05T23:32:00Z

Draft!

This PR aims to overhaul the current MS UI.

An app-wide client allows all of the contract clients to be stored (and created) once instead of every time a transaction needs to be completed.
A transaction context allows tx's to be fired easily across the app without needing to use the client.
Queries and execute messages can be saved in the client folder and reused across the app. (Componentization for reoccurring transactions!)

To Do (Backend)

Create app client
Create transaction context
Create executemsgs for app client
~~Create queries for app client~~ (Won't be necessary)
Use app client in provider
Use app client in consumer

To Do (UI)

Navigation
Transition to TailwindCSS

Will add more to the lists as I go.

ethanfrey

Thank you for this PR and sorry it took so long to find the space to review properly.

I appreciate all the new components and new transaction handling. I have a few main change requests:

The whole client/core/* files seem like unnecessary levels of abstraction around the ts-codegen client, and need maintenance every time a new method is added. This is what codegen is trying to remove. Let's get rid of those and just call the ts-codegen objects directly in the components.
This "build message" and "submit message" separation seems a bit unnatural to me. But I do see how you need it to handle toasts properly. I made one suggestion to make it a bit cleaner IMO. If you can use your method with minimal handwritten code per method that doesn't come from codegen then I will look again - maybe it is the better way
You update all the contract addresses. Did you change network? Why the redeploy? Old contracts should be usable permissionlessly. If you have a reason for the deploy, please add as a comment and also link which commit hash was there.

ethanfrey · 2022-10-18T06:56:45Z

+    meshProviderClient.contractAddress,
+  )
+
+  const msg = meshProviderComposer.unstake({


How much value does this have over calling the SigningProviderClient wrapper itself?
I like the ts-codegen APIs and a lot of people get use to them. I would not wrap them without good reason.

To be honest it's more for the sake of code being organized.

Client methods go into client/ and page content into pages/.

ethanfrey · 2022-10-18T06:57:44Z

@@ -0,0 +1,27 @@
+import type { MeshLockupClient } from 'codegen/MeshLockup.client'
+import { MeshLockupMessageComposer } from 'codegen/MeshLockup.message-composer'


Huh? Message-composer? never used those. why not just call directly?

ethanfrey · 2022-10-18T06:59:18Z

+    if (!chain) return
+    const contracts = {
+      junotestnet: [
+        this.createMeshConsumerClient,


None of these are async.

Creating the SigningCosmWasmClient is async and needs it's hooks. (what you do in connectSigning).

The rest of these helpers (eg. signingMetaStakingClient) are simple sync constructors and can be called as normal functions. No need for caching and hooks here IMO.
The

We wait for the init to be completed here:

const completed = await Promise.all(initClients)

Function won't complete until all of them have been initialized. IMO this is more efficient that individually awaiting every one of them. Also want to avoid initializing clients right before they are needed, will take some time off of queries and will also save some resources. Every MB of memory matters!

ethanfrey · 2022-10-18T07:00:43Z

@@ -0,0 +1,395 @@
+/**


Why are you using the message-composers everywhere?

You can just use the SigningClients.

Message Composers are great for when you want to eg. prepare some message for a DAO to execute. But let's just execute them now (well, send them to keplr, where it asks for use approval)

ethanfrey · 2022-10-18T07:02:15Z

@@ -1,181 +1,127 @@
-import { useWallet } from '@cosmos-kit/react';
+import { MouseEventHandler, useEffect, useMemo } from 'react'


This file is pure create-cosmos-app afaik.
I would upstream any changes here to that. Calling @pyramation

My goal here was to restyle the components here to use TailwindCSS. The original buttons are ChakraUI and include some data that is not necessary to this specific application. create-cosmos-app is the boilerplate for us to adapt to our needs :)

ethanfrey · 2022-10-18T07:03:07Z

-    meshSlasherAddr: "osmo1fewayl23e89rkejx932ljxf0ylsmtqc49r4myzzsvxcpweptmu0szshpkv"
-};
+  meshLockupAddr:
+    'osmo1nhenmvnx25v66ww0mzv6mgcyqzl963ld8cclyvyd0wqrukfnhr8s05tkkk',


Wait, why did you change the addresses here? Did you redeploy?

Jake redeployed these contracts, were missing some queries.

Yeah, we will change them again soon when I redeploy the latest.

ethanfrey · 2022-10-18T07:03:24Z

-    metaStakingAddr: "juno15tuw2fdyl666j7569ynt9dpdqykwzqw4d4sxrf0smcezudeafedsqu7n97",
-    meshConsumerAddr: "juno12e2p5rgcmcgu2t63mwystxzlfwf50kpgn9gjlnz2sf6q76pnkcjqrna22u",
-    meshConsumerPort: "wasm.juno12e2p5rgcmcgu2t63mwystxzlfwf50kpgn9gjlnz2sf6q76pnkcjqrna22u"
+  metaStakingAddr:


Same here. Where are new addresses from?

Jake redeployed these contracts, were missing some queries.

ethanfrey · 2022-10-18T07:04:36Z

+  ) => {
+    // Gas config
+    const fee = {
+      amount: coins(0, 'uosmo'),


This seems like bad overrides. I think keplr has better pre-chain defaults.
Why not remove this from the client, and just let keplr do it?

We use fee: 'auto' to defer to keplr

Pretty sure 'auto' throws an error when using signingCosmWasmCleint.broadcastTx but will look into it. You can also override the default gas in the context's options object when calling tx().

ethanfrey · 2022-10-18T07:06:14Z

+  tx: () => new Promise(() => {}),
+})
+
+export function TxProvider({ children }: { children: ReactNode }) {


Ahhh.... this is why you build messages, so you can have one submit item here.

Hmmm... I get the idea and feedback, but I still don't like all the indirection around just calling the client.

What about some function that would be called:

await doTx(async () => meshLockupClient.stake({amount: "1000", validator: "juno1swfuihwiufh"}));

It could set up the toasts and then call the passed function, capturing errors or success and doing the toasts.

I feel this is much cleaner than trying to add all kind of changes to each generation event and then submitting them in some custom way. We can avoid a lot of the hand-written client code, and rather just add a few lines around each action to ensure toasts are shown

We don't want to the toasts to show when the message hasn't been broadcast yet.
MessageComposer & then signingCosmWasmClient.broadcastTx is the only way to do this.

Think of the user flow as following:

Press button -> Keplr popup -> Broadcast -> "Broadcasting..." toast -> Success!

In my opinion this is optimal & better UX.

josefleventon added 15 commits October 5, 2022 15:02

Configure tailwindcss

0c234b1

Add headlessui & heroicons

475a7a4

Config tailwindcss & create validator component for provider page

e2e8194

Create base taiwlindcss components

292525b

Config changes

fd061ab

Add message-composer to codegen

9c2b2fa

Create app-wide client

314b848

Wallet component + more app config

6385935

Client updates + UI

c78b1a4

Fully use app client in provider page

48aaa80

Consumer page

343251d

Update contract addresses

1e3bd5f

Correctly change chain when navigating pages

f5f20a1

Fix chaintype issue

bc971a0

Integrate Leap wallet

5fd2dff

ethanfrey reviewed Oct 18, 2022

View reviewed changes

		@@ -0,0 +1,27 @@
		import type { MeshLockupClient } from 'codegen/MeshLockup.client'
		import { MeshLockupMessageComposer } from 'codegen/MeshLockup.message-composer'

		@@ -1,181 +1,127 @@
		import { useWallet } from '@cosmos-kit/react';
		import { MouseEventHandler, useEffect, useMemo } from 'react'

Conversation

josefleventon commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ethanfrey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

josefleventon commented Oct 5, 2022 •

edited

Loading