Skip to content
This repository has been archived by the owner on May 28, 2019. It is now read-only.

Monero: protobuf changes #368

Closed
4 tasks done
tsusanka opened this issue Oct 9, 2018 · 5 comments
Closed
4 tasks done

Monero: protobuf changes #368

tsusanka opened this issue Oct 9, 2018 · 5 comments

Comments

@tsusanka
Copy link
Contributor

tsusanka commented Oct 9, 2018

  • merge step 8 into step 7 or find a better name for step 8 (I think the first one is a better choice)
  • remove MoneroTransactionInitAck.in_memory, many_inputs, many_outputs?
  • rename MoneroTransactionSignInputRequest.alpha_enc to MoneroTransactionSignInputRequest.alpha ? Or maybe better MoneroTransactionSignInputRequest.pseudo_out_alpha?

  • please verify this, but MoneroRctKey message contains two fields dest and mask. dest is used for the one-time address' public key, which is fine, but mask is used for the full commitment, also when the mask is completely unknown. That's confusing, I suggest to rename it to commitment
@ph4r05
Copy link
Contributor

ph4r05 commented Oct 17, 2018

MoneroRctKey is equivalent for Monero rct::ctkey:

    //containers For CT operations
    //if it's  representing a private ctkey then "dest" contains the secret key of the address
    // while "mask" contains a where C = aG + bH is CT pedersen commitment and b is the amount
    // (store b, the amount, separately
    //if it's representing a public ctkey, then "dest" = P the address, mask = C the commitment

https://github.com/monero-project/monero/blob/5c85da5a73424017ea9c340a0412f90356d77358/src/ringct/rctTypes.h#L91

We may add a comment to the fields.

@tsusanka
Copy link
Contributor Author

Ugh, that's just dull. We use it for both "private" and "public" I guess, right?

I still suggest to rename it if possible (we can do MoneroRctKey{Private,Public}), or is it too much of a hassle?

ph4r05 added a commit to ph4r05/trezor-common that referenced this issue Oct 17, 2018
ph4r05 added a commit to ph4r05/trezor-common that referenced this issue Oct 17, 2018
@ph4r05
Copy link
Contributor

ph4r05 commented Oct 17, 2018

I've made it MoneroRctKeyPublic as in the protocol we use only commitment variant.

Once the trezor/trezor-common#216 gets merged I would need version bump in python-trezor also.

I have already finished modifications to both trezor-core PR and monero-agent so after trezor-common is ready I can apply those changes. Then I will also update monero-doc and monero/readme.md

ph4r05 added a commit to ph4r05/trezor-common that referenced this issue Oct 18, 2018
@tsusanka
Copy link
Contributor Author

I can close this, right?

@ph4r05
Copy link
Contributor

ph4r05 commented Oct 23, 2018

ok for me

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants