Skip to content

Set up Diem ID Domains on mini wallet startup - #306

Merged
sunmilee merged 7 commits into
diem:masterfrom
sunmilee:miniwallet
Jun 16, 2021
Merged

Set up Diem ID Domains on mini wallet startup#306
sunmilee merged 7 commits into
diem:masterfrom
sunmilee:miniwallet

Conversation

@sunmilee

@sunmilee sunmilee commented Jun 1, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor
  1. Add domains to parent vasp account on-chain during wallet startup
  2. Read events to create a domain id -> on-chain address mapping
  3. Add payment metadata function

@sunmilee
sunmilee marked this pull request as ready for review June 1, 2021 23:17
@sunmilee
sunmilee requested a review from xli June 1, 2021 23:18
@sunmilee
sunmilee force-pushed the miniwallet branch 3 times, most recently from 324eec0 to 6d23f01 Compare June 2, 2021 02:32
xli
xli reviewed Jun 2, 2021
View reviewed changes
Comment thread src/diem/testing/cli.py Outdated
Comment thread src/diem/testing/cli.py Outdated
Comment thread src/diem/testing/cli.py Outdated
Comment thread src/diem/testing/miniwallet/app/app.py Outdated
Comment thread src/diem/testing/miniwallet/app/event_puller.py Outdated
Comment thread src/diem/testing/miniwallet/app/event_puller.py Outdated
Comment thread src/diem/testing/miniwallet/config.py Outdated
Comment thread src/diem/testnet.py Outdated
Comment thread src/diem/testnet.py Outdated
Comment thread src/diem/testing/miniwallet/app/models.py Outdated
@sunmilee
sunmilee force-pushed the miniwallet branch 4 times, most recently from 154e987 to 16be7b9 Compare June 4, 2021 01:58
@sunmilee
sunmilee force-pushed the miniwallet branch 3 times, most recently from c4b2845 to 9762ad4 Compare June 10, 2021 06:07
@sunmilee
sunmilee requested a review from xli June 11, 2021 09:26
xli
xli reviewed Jun 11, 2021
View reviewed changes
Comment thread src/diem/jsonrpc/client.py Outdated
Comment thread src/diem/utils.py Outdated
Comment thread src/diem/jsonrpc/client.py Outdated
Comment thread src/diem/testing/miniwallet/config.py Outdated
Comment thread src/diem/testing/suites/conftest.py Outdated
Comment thread src/diem/testing/suites/conftest.py Outdated
def start_stub_wallet(diem_client: jsonrpc.Client) -> Tuple[AppConfig, App]:
conf = AppConfig(name="stub-wallet", server_conf=ServerConfig(**dmw_stub_server()))
conf = AppConfig(
name="stubwallet", server_conf=ServerConfig(**dmw_stub_server()), diem_id_domain=dmw_stub_diem_id_domain()

@xli xli Jun 11, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we probably should keep original name "stub-wallet", don't see a reason to rename it.

Comment thread mini-wallet.md Outdated
Comment thread tests/miniwallet/conftest.py Outdated
Comment thread tests/miniwallet/conftest.py Outdated
@xli

xli commented Jun 11, 2021

Copy link
Copy Markdown

I suggest we split this PR into:

  1. generate new code from diem core for new PaymentMetadata and other data structures. this PR should be easy to review and land as you only include generated code and maybe txnmetadata.py change (add a test for it)
  2. change to jsonrpc/client get_diem_id_domain_map, and faucet#mint, also need add a test for them.
  3. add an option to testing/cli.py for customizing the diem-id-domain, including test_testing_cli to confirm it is good.
  4. miniwallet changes, generate dynamic diem id domain for starting app in test env, create a test to confirm the diem id is handled properly. current implementation only handle the reference id set on txn.

@xli

xli commented Jun 11, 2021

Copy link
Copy Markdown

I suggest we split this PR into:

  1. generate new code from diem core for new PaymentMetadata and other data structures. this PR should be easy to review and land as you only include generated code and maybe txnmetadata.py change (add a test for it)
  2. change to jsonrpc/client get_diem_id_domain_map, and faucet#mint, also need add a test for them.
  3. add an option to testing/cli.py for customizing the diem-id-domain, including test_testing_cli to confirm it is good.
  4. miniwallet changes, generate dynamic diem id domain for starting app in test env, create a test to confirm the diem id is handled properly. current implementation only handle the reference id set on txn.

Or combine first 3 together, as they are pretty much finished in this PR.

@sunmilee
sunmilee requested a review from xli June 14, 2021 20:55
xli
xli reviewed Jun 14, 2021
View reviewed changes
Comment thread src/diem/testnet.py Outdated
Comment thread tests/miniwallet/conftest.py Outdated
Comment thread src/diem/testing/suites/conftest.py Outdated
xli
xli reviewed Jun 15, 2021
View reviewed changes
Comment thread src/diem/testing/suites/conftest.py Outdated
@sunmilee
sunmilee requested a review from xli June 15, 2021 22:42
@sunmilee
sunmilee force-pushed the miniwallet branch 2 times, most recently from a8cb4df to 8e4f53e Compare June 15, 2021 22:45
xli
xli reviewed Jun 16, 2021
View reviewed changes
Comment thread src/diem/testing/cli.py Outdated
Comment thread src/diem/testing/cli.py Outdated
Comment thread src/diem/testing/cli.py Outdated
Comment thread src/diem/testing/cli.py Outdated
Comment thread src/diem/testing/suites/conftest.py Outdated
xli
xli reviewed Jun 16, 2021
View reviewed changes
Comment thread src/diem/jsonrpc/client.py
@sunmilee
sunmilee merged commit cc1a00d into diem:master Jun 16, 2021
This was referenced Jun 22, 2021
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@xli xli xli approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sunmilee @xli