-
Notifications
You must be signed in to change notification settings - Fork 32
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
Further Bitshares operations support #207
Conversation
Alright, every operation now has an untested basic implementation in the visualize function. Will need to create beet-js examples to test each, it's likely that many of the op fields aren't strings. Some of these operation types are quite high risk or rather have permanent consequences for a single button click, we should perhaps increase the steps required for more destructive actions, e.g. either acknowledge the risk with an additional warning before enabling the approve button? We could also introduce the account blocklist warnings for some of these operations, warning them to for example not transfer their account to scam account, etc. |
How should we display the byte data in operation 35 (custom)? We could convert it to like base64 instead of displaying the bytes as a string (will look horrible). Alternatively we could just display the length/size of the data field to show it's populated to the user?
|
Data in the final JSON string should be in HEX format, E.G. To make it more advanced (in the future), maybe beet can try to guess what's in the byte array. It's very arbitrary anyway. One use case is to store additional account data (see bitshares/bitshares-core#1926), currently used by the BitShares Mobile app (also see https://cryptofresh.com/posts). The Moneda Par app use the operation to store JSON strings. Some other people store pure (printable) text, etc. |
d468adf
to
e54fd5a
Compare
Currently there's a bug in the transaction request - it's hardcoded to look for account id in the incoming request, however is can go by other fields like seller & from. This is somewhat solved in the protocol+qr pull request by just referencing the currently selected account, however this could differ from the proposal which may refer to one of your other accounts in the wallet. Perhaps we will need to extract this 'from account id' from all possible transaction types & then fall back on using the currently selected wallet account? |
FWIW in bitshares-core there is a |
In pull request #213 I implemented a There's still an open review in #213 re implementing sufficient checks prior to prompt visualization, however at the moment I'm planning on bringing that solution into this PR when merged. |
Placeholders for all Bitshares operations
Check if account ids are blocked in injected calls Refactor getblocklist to only fetch blocklist Some operation field fixes
Closing in favour of #222 |
To address the following issue: #198
This PR intends to support every Bitshares operation within the visualize functions at the very least.
Examples for each operation will need to be created in the beet-js repo to test these changes.
Still a work in progress.