-
Notifications
You must be signed in to change notification settings - Fork 110
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
Add cosmwasm msgs #158
Add cosmwasm msgs #158
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
🏄
<div>{printableCoins(msgValue.funds, chain)}</div> | ||
</li> | ||
<li> | ||
<JsonEditor readOnly content={{ json: JSON.parse(fromUtf8(msgValue.msg, true)) }} /> |
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.
What's the reason for using lossy mode (true
) here? I think if the contents of the variable is not valid UTF-8, something went wrong and we should throw.
</li> | ||
<li> | ||
<label>Salt:</label> | ||
<div>{fromUtf8(msgValue.salt, true)}</div> |
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.
This might be non-printable. I'd show it as hex.
<div>{fromUtf8(msgValue.salt, true)}</div> | |
<div>{toHex(msgValue.salt)}</div> |
// The JsonEditor does not escape \n or remove whitespaces, so we need to parse + stringify | ||
return toUtf8(JSON.stringify(JSON.parse(msgContent))); | ||
} catch { | ||
return Uint8Array.from([]); |
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 it would be better to log those errors. What kind of exceptions are you observing here?
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.
Those errors would be logged in the json editor itself. This code is to make sure that it does not throw while the json is being edited and to provide a fallback value to the msgValue.msg
below. I guess it could be undefined
instead of Uint8Array.from([])
since it uses fromPartial()
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.
Ah, okay, I see. undefined
and empty bytes are the same thing at protobuf level, so using that does not help much.
Can we prevent that invalid JSON can be submitted? It would be bad if a user creates an empty msg because they have some typo in their JSON.
const [codeId, setCodeId] = useState(""); | ||
const [label, setLabel] = useState(""); | ||
const [adminAddress, setAdminAddress] = useState(""); | ||
const [fixMsg, setFixMsg] = useState(false); |
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.
This should always be false. The user must not change that. https://medium.com/cosmwasm/dev-note-3-limitations-of-instantiate2-and-how-to-deal-with-them-a3f946874230 explains why
value={String(fixMsg)} | ||
onChange={({ target }) => setFixMsg(target.checked)} | ||
/> | ||
</div> |
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.
As mentioned above, let's remove this UI element and just set it to false
value={salt} | ||
onChange={({ target }) => setSalt(target.value)} | ||
/> | ||
</div> |
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.
Here we should accept binary data. I'd make this a hex input. Maybe we can set a placeholder with "Salt (hex encoded)"?
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.
Beautiful!
Part of #99.
Updates lockfile to v3, now that CI uses node@18 and therefore npm@9.
Updates cosmjs to latest.
Successfully tested Instantiate, Instantiate2, Execute, and Migrate with CW20.