diem / dip

Libra Improvement Proposals
https://lip.libra.org
Apache License 2.0
40 stars 55 forks source link

Update Recipient Signature generation reference code in python #69

Closed longbowlu closed 3 years ago

longbowlu commented 3 years ago

I've verified this snippet of code against testnet

n4ss commented 3 years ago

I think it will be easier to read if we keep it very explicit. wdyt of:

from pylibra import lcs
from pylibra import libra_types as libra
from pylibra import serde_types as st
...

metadata: Metadata = libra.Metadata__TravelRuleMetadata(
  libra.TravelRuleMetadata__TravelRuleMetadataVersion0(
    libra.TravelRuleMetadataV0(
      off_chain_reference_id="reference_id",
    )
  )
)
// `lcs_metadata` will be used as the `metadata` parameter in encode_peer_to_peer_with_metadata_script()
lcs_metadata = lcs.serialize(metadata, libra.Metadata)

dual_attest_msg_bytes: bytearray = bytearray()
dual_attest_msg_bytes.extend(lcs_metadata)  # Travel Rule metadata
dual_attest_msg_bytes.extend(lcs.serialize(bytes.fromhex("793251de1a61e9b4d1a17d13aa015e45"), bytes))  # Raw on-chain address (https://github.com/libra/lip/blob/master/lips/lip-5.md#terminology)
dual_attest_msg_bytes.extend(lcs.serialize(st.uint64(1000000), st.uint64))  # Amount
dual_attest_msg_bytes.extend(b"@@$$LIBRA_ATTEST$$@@")  # ASCII-encoded Libra Domain Separator string

// We sign the bytes directly with the compliance private key
// `receiver_metadata_signature` will be used as the `metadata_signature` parameter in  encode_peer_to_peer_with_metadata_script()
receiver_metadata_signature = compliance_private_key.sign(dual_attest_msg_bytes)
bors-libra commented 3 years ago

:exclamation: Invalid command

Bors help and documentation
Bors is a Github App used to manage merging of PRs in order to ensure that CI is always green. It does so by maintaining a [Merge Queue](https://github.com/libra/lip/projects/1). Once a PR reaches the head of the Merge Queue it is rebased on top of the latest version of the PR's `base-branch` (generally `master`) and then triggers CI. If CI comes back green the PR is then merged into the `base-branch`. Regardless of the outcome, the next PR is the queue is then processed. ### General - This project's Merge Queue can be found [here](https://github.com/libra/lip/projects/1). - This project requires PRs to be [Reviewed](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-reviews) and Approved before they can be queued for merging. - Before PR's can be merged they must be configured with ["Allow edits from maintainers"](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork) enabled. This is needed for Bors to be able to update PR's in-place so that Github can properly recognize and mark them as "merged" once they've been merged into the upstream branch. ### Commands Bors actions can be triggered by posting a comment which includes a line of the form `/`. | Command | Action | Description | | --- | --- | --- | | __Land__ | `land`, `merge` | attempt to land or merge a PR | | __Canary__ | `canary`, `try` | canary a PR by performing all checks without merging | | __Cancel__ | `cancel`, `stop` | stop an in-progress land | | __Cherry Pick__ | `cherry-pick ` | cherry-pick a PR into `` branch | | __Priority__ | `priority` | set the priority level for a PR (`high`, `normal`, `low`) | | __Help__ | `help`, `h` | show this help message | ### Options Options for Pull Requests are configured through the application of labels. |           Option           | Description | | --- | --- | | ![label: bors-high-priority](https://img.shields.io/static/v1?label=&message=bors-high-priority&color=lightgrey) | Indicates that the PR is high-priority. When queued the PR will be placed at the head of the merge queue. | | ![label: bors-low-priority](https://img.shields.io/static/v1?label=&message=bors-low-priority&color=lightgrey) | Indicates that the PR is low-priority. When queued the PR will be placed at the back of the merge queue. | | ![label: bors-squash](https://img.shields.io/static/v1?label=&message=bors-squash&color=lightgrey) | Before merging the PR will be squashed down to a single commit, only retaining the commit message of the first commit in the PR. |
longbowlu commented 3 years ago

I think it will be easier to read if we keep it very explicit. wdyt of:

from pylibra import lcs
from pylibra import libra_types as libra
from pylibra import serde_types as st
...

metadata: Metadata = libra.Metadata__TravelRuleMetadata(
  libra.TravelRuleMetadata__TravelRuleMetadataVersion0(
    libra.TravelRuleMetadataV0(
      off_chain_reference_id="reference_id",
    )
  )
)
// `lcs_metadata` will be used as the `metadata` parameter in encode_peer_to_peer_with_metadata_script()
lcs_metadata = lcs.serialize(metadata, libra.Metadata)

dual_attest_msg_bytes: bytearray = bytearray()
dual_attest_msg_bytes.extend(lcs_metadata)  # Travel Rule metadata
dual_attest_msg_bytes.extend(lcs.serialize(bytes.fromhex("793251de1a61e9b4d1a17d13aa015e45"), bytes))  # Raw on-chain address (https://github.com/libra/lip/blob/master/lips/lip-5.md#terminology)
dual_attest_msg_bytes.extend(lcs.serialize(st.uint64(1000000), st.uint64))  # Amount
dual_attest_msg_bytes.extend(b"@@$$LIBRA_ATTEST$$@@")  # ASCII-encoded Libra Domain Separator string

// We sign the bytes directly with the compliance private key
// `receiver_metadata_signature` will be used as the `metadata_signature` parameter in  encode_peer_to_peer_with_metadata_script()
receiver_metadata_signature = compliance_private_key.sign(dual_attest_msg_bytes)

Seems like the only difference of this and the original code is DOMAIN_SEPARATOR is not serialized? I tried this and it did not work. Did you test this snippet in testnet ? @n4ss

n4ss commented 3 years ago

Differences are: domain separator is not serialized and the byte vector is not serialized either. This is the same as your implem (lcs-encoding of the struct is lcs-encoding of each field) and matches the ref this time:

        let message = metadata;
        Vector::append(&mut message, LCS::to_bytes(&payer));
        Vector::append(&mut message, LCS::to_bytes(&deposit_value));
        Vector::append(&mut message, DOMAIN_SEPARATOR);
        message

https://github.com/libra/libra/blob/906bfb40984538ca3773ab5c2aaa911aab517b8b/language/stdlib/modules/DualAttestation.move#L325

longbowlu commented 3 years ago

lcs.serialize(bytes.fromhex("793251de1a61e9b4d1a17d13aa015e45"), bytes) is not the correct serialization for address. I think it's because the address has a different type than normal bytes. You can get the lcs bytes by calling utils.account_address(addr_hex).lcs_serialize()

and If your point is dataclass is less clear to read, I've updated to also include a version that concats individually serialized bytes (very similar to the original code snippet before this change)