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 move resources and scripts for DiemID #8336

Merged
merged 10 commits into from
May 12, 2021
Merged

Conversation

sunmilee
Copy link

@sunmilee sunmilee commented May 5, 2021

Motivation

Adds resources and scripts needed for DiemID domain management on chain, based on DIP-10: https://github.com/diem/dip/blob/main/dips/dip-10.md

DiemId.move defines all the resources and functions:

  • Add resources DiemIdDomains, DiemIdDomain, DiemIdDomainEvent, DiemIdDomainManager
  • TC add/remove diemIdDomain
  • Event for add/remove of diemIdDomain emitted and saved to DomainEventManager
  • DiemAccount publishes diemIdDomain resource using create_parent_vasp_account

Scripts:

  • Script in AccountAdministrationScripts.move to add diemIdDomain resource to existing parentvasps
  • Script in TreasuryComplianceScripts.move to update diemIdDomain resource for parentvasps

Test Plan

Functional tests under diem_id_domain.move
build: cargo run
tests: cargo test
cargo x test -p move-lang-functional-tests diem_id_domain.move
language/tools/move-cli and set UPDATE_BASELINE=1 and run cargo test in there. It should update the failing test's .exp file.

@bors-libra bors-libra added this to In Review in bors May 5, 2021
@sunmilee sunmilee force-pushed the diemidmove branch 6 times, most recently from 25c56e4 to 03c5e27 Compare May 7, 2021 19:23
@sunmilee sunmilee marked this pull request as ready for review May 7, 2021 19:32
@sunmilee sunmilee mentioned this pull request May 7, 2021
24 tasks
@sunmilee sunmilee requested a review from tzakian May 7, 2021 19:33
@sunmilee sunmilee force-pushed the diemidmove branch 2 times, most recently from da2f35c to dbb7332 Compare May 7, 2021 19:43
Copy link
Contributor

@sblackshear sblackshear left a comment

Choose a reason for hiding this comment

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

Great stuff!!

language/diem-framework/modules/DiemId.move Outdated Show resolved Hide resolved
language/diem-framework/modules/DiemId.move Show resolved Hide resolved
language/diem-framework/modules/DiemId.move Outdated Show resolved Hide resolved
language/diem-framework/modules/DiemId.move Show resolved Hide resolved
language/diem-framework/modules/DiemId.move Outdated Show resolved Hide resolved
@sunmilee sunmilee force-pushed the diemidmove branch 3 times, most recently from 11e9e94 to d1a2d25 Compare May 11, 2021 04:00
@sunmilee
Copy link
Author

/land

@bors-libra bors-libra moved this from In Review to Queued in bors May 12, 2021
@bors-libra
Copy link
Contributor

❗ Land has been canceled due to this PR being updated with new commits. Please issue another Land command if you want to requeue this PR.

@bors-libra bors-libra moved this from Queued to In Review in bors May 12, 2021
@sunmilee
Copy link
Author

/land

@bors-libra bors-libra moved this from In Review to Queued in bors May 12, 2021
@bors-libra bors-libra moved this from Queued to Testing in bors May 12, 2021
@sunmilee
Copy link
Author

/land

@bors-libra
Copy link
Contributor

@sunmilee 💡 This PR is already queued for landing

@github-actions
Copy link

Cluster Test Result

Test runner setup time spent 264 secs
Compatibility test results for land_69f35022 ==> land_4cdab196 (PR)
1. All instances running land_69f35022, generating some traffic on network
2. First full node land_69f35022 ==> land_4cdab196, to validate new full node to old validator node traffic
3. First Validator node land_69f35022 ==> land_4cdab196, to validate storage compatibility
4. First batch validators (14) land_69f35022 ==> land_4cdab196, to test consensus and traffic between old full nodes and new validator node
5. First batch full nodes (14) land_69f35022 ==> land_4cdab196
6. Second batch validators (15) land_69f35022 ==> land_4cdab196, to upgrade rest of the validators
7. Second batch of full nodes (15) land_69f35022 ==> land_4cdab196, to finish the network upgrade, time spent 682 secs
all up : 872 TPS, 5202 ms latency, 6000 ms p99 latency, no expired txns, time spent 250 secs
Logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-05-12T21:56:19Z',to:'2021-05-12T22:18:05Z'))
Dashboard: http://grafana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/d/performance/performance?from=1620856579000&to=1620857885000
Validator 1 logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-05-12T21:56:19Z',to:'2021-05-12T22:18:05Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

./scripts/cti --tag land_69f35022 --cluster-test-tag land_4cdab196 -E BATCH_SIZE=15 -E UPDATE_TO_TAG=land_4cdab196 --report report.json --suite land_blocking_compat

🎉 Land-blocking cluster test passed! 👌

@bors-libra bors-libra removed this from Testing in bors May 12, 2021
@bors-libra bors-libra merged commit 4cdab19 into diem:main May 12, 2021
@bors-libra bors-libra temporarily deployed to Docker May 12, 2021 22:18 Inactive
@bors-libra bors-libra temporarily deployed to Sccache May 12, 2021 22:18 Inactive
zgfzgf pushed a commit to zgfzgf/diem that referenced this pull request Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants