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

Add root hash as option parameters w/ support custom raw transaction for contract interact.. #6

Closed
wants to merge 4 commits into from

Conversation

feliciss
Copy link

No description provided.

@codecov-io
Copy link

codecov-io commented Jan 13, 2020

Codecov Report

Merging #6 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #6   +/-   ##
=======================================
  Coverage   75.29%   75.29%           
=======================================
  Files           2        2           
  Lines          85       85           
=======================================
  Hits           64       64           
  Misses         14       14           
  Partials        7        7

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6caf195...d0ebcdb. Read the comment docs.

@feliciss feliciss changed the title Add root hash as option parameters. Add root hash as option parameters w/ support custom raw transaction for contract interact.. Jan 13, 2020
@the729
Copy link
Owner

the729 commented Jan 15, 2020

Thank you for the PR. I have some suggestions:

  1. Could you please separate this into 2 PRs? One for configuration, one for submitting raw txns. So that we can discuss them separately.

  2. For the configuration part, we have already discussed in [Options] Need option for customized local node genesis hash. #5 . Also in my understanding, the name RootHash should be GenesisHash or GenesisTxnHash everywhere. Root hash in Libra refers to the root of Merkle tree accumulator, which is updated when each new txn is accumulated. When the world begins, after the genesis txn is submitted, the MT accumulator has only 1 txn. That is when the root hash equals to the genesis txn hash.

  3. For the raw txn part, I'm not sure if gopherJS binding can correctly convert a struct as complex as types.TransactionPayload from js to golang. Could you please have a try? Probably you could add an example to submit a p2p transaction with the newly added submitContractTransaction API?

@feliciss
Copy link
Author

feliciss commented Jan 16, 2020

Thank you for the PR. I have some suggestions:

  1. Could you please separate this into 2 PRs? One for configuration, one for submitting raw txns. So that we can discuss them separately.
  2. For the configuration part, we have already discussed in [Options] Need option for customized local node genesis hash. #5 . Also in my understanding, the name RootHash should be GenesisHash or GenesisTxnHash everywhere. Root hash in Libra refers to the root of Merkle tree accumulator, which is updated when each new txn is accumulated. When the world begins, after the genesis txn is submitted, the MT accumulator has only 1 txn. That is when the root hash equals to the genesis txn hash.
  3. For the raw txn part, I'm not sure if gopherJS binding can correctly convert a struct as complex as types.TransactionPayload from js to golang. Could you please have a try? Probably you could add an example to submit a p2p transaction with the newly added submitContractTransaction API?
  1. Yes.
  2. Okay, will perform basic search and try to understand.
  3. Yes. It's failed in gopher-js:
Error: cannot internalize types.TransactionPayload.

@the729
Copy link
Owner

the729 commented Jan 17, 2020

Since I have pushed several commits yesterday (to fix compatibility with the latest testnet), could you please rebase your branch, so that my commits do not show up?

Rethink the raw txn format.
Add to gopher-js.
@feliciss
Copy link
Author

Have rewritten and checked both NewRawCustomModuleTransaction and NewRawCustomScriptTransaction in browser.
NewRawCustomModuleTransaction works as fine, while the NewRawCustomScriptTransaction requires a nested enum type that gopher-js can't translate and understand it to javascript.

Result as shown below:

Error: cannot internalize types.TransactionArgument

Could you please have a different options for constructing a TxnPayloadScript's args and support the arguments passed in gopher-js, if you are available?

@the729
Copy link
Owner

the729 commented Jan 28, 2020

Close. See #4

@the729 the729 closed this Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants