-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix: Repair send transaction fields and add call and decoder #146
Conversation
Deployed with Cloudflare Pages ☁️ 🚀 🆗 |
5c199c8
to
358919f
Compare
src/components/shared/ParamField.vue
Outdated
import LSP4DigitalAsset from '@erc725/erc725.js/schemas/LSP4DigitalAsset.json' | ||
import LSP9Vault from '@erc725/erc725.js/schemas/LSP9Vault.json' | ||
|
||
const schemas = LSP3ProfileMetadata.concat(LSP4DigitalAsset, LSP9Vault) |
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.
Would suggest writing it as follow for readability (easier to see the schemas in there when they are written line by line. But not a big deal.
const schemas = [
...LSP3ProfileMetadata,
...LSP4DigitalAsset,
...LSP9Vault
]
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.
Changing
src/components/shared/ParamField.vue
Outdated
value = numberToHex(value) | ||
} | ||
const value = | ||
typeof _value === 'string' && /^0x/.test(_value) |
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.
is it to test if it starts with?
Maybe it can be re-written like this?
typeof _value === 'string' && /^0x/.test(_value) | |
typeof _value === 'string' && _value.startsWith("0x") |
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, yes.
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, though haven't tested manually yet.
Clickup
https://app.clickup.com/t/2645698/DEV-10497
Task
Repair SendTransaction panel
Add call button and call decoder.
Properly encoding bytes32 as a LSP data key if checkbox is on
Properly working with bytes and bytes32 arrays.
Allowing Call and allowing data conversion (here bytes) and LSP decoding (here LSP3Profile)