Skip to content
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

[2] Add raw json for operations and objects #2657

Open
4 tasks
abitmore opened this issue May 4, 2019 · 31 comments · Fixed by #2743 or #2996
Open
4 tasks

[2] Add raw json for operations and objects #2657

abitmore opened this issue May 4, 2019 · 31 comments · Fixed by #2743 or #2996
Assignees
Labels
[1b] User Story The User Story details a requirement. It may ref a parent Project (Epic). It may ref child Task(s) [3] Feature Classification indicating the addition of novel functionality to the design [5b] Small Indicates size of task. Est. between one and two hours [7] Estimated Administration flag, notifies hours are estimated and requires revisiting

Comments

@abitmore
Copy link
Member

abitmore commented May 4, 2019

Is your feature request related to a problem? Please describe.
image

In some cases it's not clear what the ops means.
Pages that I thought of:

  • block explorer
  • account activities
  • proposed proposals
  • transaction confirmation page

Describe the solution you'd like
Add a link so the nerds can check the raw json (plain text, best formatted well), contains elements described in #2657 (comment)

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@startailcoon startailcoon added [1b] User Story The User Story details a requirement. It may ref a parent Project (Epic). It may ref child Task(s) [3] Feature Classification indicating the addition of novel functionality to the design [5b] Small Indicates size of task. Est. between one and two hours [7] Estimated Administration flag, notifies hours are estimated and requires revisiting labels May 6, 2019
@startailcoon startailcoon added this to the 190524 milestone May 6, 2019
@startailcoon startailcoon changed the title Add raw json for operations and objects [2] Add raw json for operations and objects May 6, 2019
@startailcoon
Copy link
Contributor

Good idea, it shouldn't be difficult to view and verify the json transaction, which is just what the blockchain is all about.

  • "Click to view Raw TX" type of text
  • Should view the raw json in a new modal window.
  • On all TX and Blockchain OPs viewed in app

@OpenLedgerApp
Copy link

@startailcoon Could you please clarify the following:

What info should be in the raw json? this object or trx object or smth else?
image

How should it be displayed?like this?
image

@startailcoon
Copy link
Contributor

@OpenLedgerApp Exactly the spot. It's mainly to make it possible to view under the "Explorer -> View Block" section rather than when you make a new TX, which you probably know what it does.

The Raw TX should view the JSON object for the TX in question, just like when we don't have a pretty view for an operation. You can view how we do on that when we have no match for a "TX Translation".

I imagine it something like this, to make it visible but non intrusive. Much like your example above.
It should open a modal to view the JSON object.

image

@startailcoon startailcoon changed the title [2] Add raw json for operations and objects [2][OpenLedgerApp] Add raw json for operations and objects May 31, 2019
@startailcoon startailcoon changed the title [2][OpenLedgerApp] Add raw json for operations and objects [2] Add raw json for operations and objects May 31, 2019
@OpenLedgerApp
Copy link

@startailcoon ok, thanks for clarifying. Can i claim this one?

@startailcoon
Copy link
Contributor

@OpenLedgerApp it's already assigned to you, so feel free to work and push a PR on it.

OpenLedgerApp pushed a commit to OpenLedgerApp/bitshares-ui that referenced this issue Jun 3, 2019
@sschiessl-bcp sschiessl-bcp added this to To do in 191120 Release via automation Jun 27, 2019
@clockworkgr clockworkgr modified the milestones: 190607, 190705 Jun 28, 2019
OpenLedgerApp pushed a commit to OpenLedgerApp/bitshares-ui that referenced this issue Jun 28, 2019
OpenLedgerApp pushed a commit to OpenLedgerApp/bitshares-ui that referenced this issue Jun 28, 2019
# Conflicts:
#	app/assets/stylesheets/layout/_page_layout.scss
OpenLedgerApp pushed a commit to OpenLedgerApp/bitshares-ui that referenced this issue Jun 28, 2019
@startailcoon startailcoon modified the milestones: 190705, 190719 Jul 16, 2019
@abitmore abitmore reopened this Aug 9, 2019
191120 Release automation moved this from Done to In progress Aug 9, 2019
@abitmore
Copy link
Member Author

abitmore commented Aug 9, 2019

@sschiessl-bcp see comments in #3001 (comment).

@abitmore
Copy link
Member Author

abitmore commented Aug 9, 2019

@sschiessl-bcp by the way the modal in #2996 is NOT what I want. I need a JSON, not a tree.

@abitmore
Copy link
Member Author

abitmore commented Aug 9, 2019

If you must have a tree in the module, please make a button to "expand all".

@sschiessl-bcp
Copy link
Contributor

Let's do copyable text in #3020. I consider this issue completed with the tree that allows inspection. Sorry if we misunderstood the intent of your request.

@abitmore
Copy link
Member Author

abitmore commented Aug 9, 2019

It's clearly stated in OP, this issue is for adding "raw json". Unless you don't know what is a raw json.

@abitmore
Copy link
Member Author

abitmore commented Aug 9, 2019

A well formatted JSON is much better than the appearance currently showing in UI.

@sschiessl-bcp
Copy link
Contributor

"raw json"

For the times I used this popup the "tree json" was sufficient for me and I didn't think more about it. You want plaintext. I put a question for you in the follow up issue.

@abitmore
Copy link
Member Author

abitmore commented Aug 9, 2019

Please redo this issue. Current implementation is not acceptable.

  • the tree in account activities doesn't include important data
    • id of the history
    • block_num
    • trx_in_block
    • op_in_trx
    • virtual_op
  • the trees in block explorer, I just don't want to use it.
    • I need a raw json of the whole block
    • btw, the "?" icon added near the fee in that page just doesn't make sense
  • proposed transactions
    • I need the json about the proposal, but not one of the operations in the proposal
      • id of the proposal
      • proposer
      • required approvals
      • added approvals
      • when will it expire
      • review period
  • confirmation page, I guess you've got what I want, see [1.25] Add copy plaintext feature for offline signing to Transaction Confirm #3020. But I still think it's better to redo it in this issue. Let that issue be for beet only, which is more complicated, and could take forever to complete.

Last but not least, I don't think it's a good practice to auto-close issues without the issue-creators' confirmation. I'm able to reopen issues that closed by developers because I'm an admin, but other normal users can not, it's super annoying, because nobody pays attention to closed issues, if they reply in the issue their messages would easily get lost, in in that case they have to create new issues.

@sschiessl-bcp sschiessl-bcp modified the milestones: 190802, 190816 Aug 11, 2019
@startailcoon startailcoon modified the milestones: 190816, 190830 Aug 17, 2019
@sschiessl-bcp sschiessl-bcp removed this from the 190830 milestone Nov 4, 2019
@sschiessl-bcp
Copy link
Contributor

I'm able to reopen issues that closed by developers because I'm an admin, but other normal users can not, it's super annoying, because nobody pays attention to closed issues

I can agree to that when the issue creator is a known to be active.

@sschiessl-bcp sschiessl-bcp removed this from In progress in 191120 Release Nov 8, 2019
@abitmore
Copy link
Member Author

abitmore commented Feb 15, 2022

add a link on the transaction confirmation page to preview the transaction in JSON format

@syalon recommended that we can add a QR code for the JSON so that it can be signed and broadcast via a mobile app. @sschiessl-bcp @xiangxn what do you think? (Also see #3020)

@abitmore abitmore assigned xiangxn and unassigned OpenLedgerApp Feb 15, 2022
@sschiessl-bcp
Copy link
Contributor

add a link on the transaction confirmation page to preview the transaction in JSON format

@syalon recommended that we can add a QR code for the JSON so that it can be signed and broadcast via a mobile app. @sschiessl-bcp @xiangxn what do you think? (Also see #3020)

Idea is to be able to click to the "confirm transaction" popup without unlocking and then be able to switch to a QR code?

@abitmore
Copy link
Member Author

add a link on the transaction confirmation page to preview the transaction in JSON format

@syalon recommended that we can add a QR code for the JSON so that it can be signed and broadcast via a mobile app. @sschiessl-bcp @xiangxn what do you think? (Also see #3020)

Idea is to be able to click to the "confirm transaction" popup without unlocking and then be able to switch to a QR code?

I'm not sure if I got what your said. Probably the most wanted behavior is to be able to construct transactions (be able to "claim" or "act as" an account) without login in the first place at all, but this looks like a big change (see also: #1285). The first and (I guess) easier step is to add a link/button to show a QR code (and as mentioned in #3020, a JSON string) on the existing transaction confirmation popup.

@xiangxn
Copy link
Contributor

xiangxn commented Feb 18, 2022

I'm guessing this is to create a transaction in the browser and sign it with the app and broadcast it. Simple transfer transactions are feasible with QR codes, but some transaction data is very large and may not be suitable for QR codes.

@abitmore
Copy link
Member Author

I'm guessing this is to create a transaction in the browser and sign it with the app and broadcast it. Simple transfer transactions are feasible with QR codes, but some transaction data is very large and may not be suitable for QR codes.

Yes it is a valid argument. I think a simple solution is to show an error if the data is too big to fit in a QR code.

Input mode Max. characters Bits/char. Possible characters, default encoding
Numeric only 7,089 31⁄3 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
Alphanumeric 4,296 51⁄2 0–9, A–Z (upper-case only), space, $, %, *, +, -, ., /, :
Binary/byte 2,953 8 ISO 8859-1
Kanji/kana 1,817 13 Shift JIS X 0208

References:

In addition, I think we need to encode the data in UTF-8? E.G. an asset_create_operation or an asset_update_operation. In this case, the maximum supported length is probably even smaller.

@xiangxn
Copy link
Contributor

xiangxn commented Feb 19, 2022

If it can't support all transactions, then this feature is not very meaningful.

@abitmore
Copy link
Member Author

Most of common transactions can be done with this, so I think it is worth doing.

@xiangxn
Copy link
Contributor

xiangxn commented Feb 20, 2022

So, is the QR code for the entire transaction required here, or just the OP's QR code?

@abitmore
Copy link
Member Author

So, is the QR code for the entire transaction required here, or just the OP's QR code?

The transaction.

sschiessl-bcp added a commit that referenced this issue Apr 18, 2022
* fix bug #3447

* fix appveyor issue

* fix github action bug

* fix github action bug

* add tr qrcode in TransactionConfirm

* add qr icon and adjust positioning

Co-authored-by: Stefan <toleantech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[1b] User Story The User Story details a requirement. It may ref a parent Project (Epic). It may ref child Task(s) [3] Feature Classification indicating the addition of novel functionality to the design [5b] Small Indicates size of task. Est. between one and two hours [7] Estimated Administration flag, notifies hours are estimated and requires revisiting
Projects
None yet
6 participants