[breaking][libra framework] add event on account creation #6156

sblackshear · 2020-09-23T05:26:37Z

New CreateAccountEvent struct that records the created address
Had to refactor LibraAccount::initialize() a bit to ensure that we can emit an event for the creation of LibraRoot
Had to refactor the event verification part of VM genesis since the number of events now depends on the # of created accounts
Added tests for each of the create_*_account functions
Added Rust wrapper for CreateAccountEvent

tzakian · 2020-09-23T15:49:43Z

language/stdlib/modules/LibraAccount.move

-
-        assert(
-            !exists<AccountOperationsCapability>(CoreAddresses::LIBRA_ROOT_ADDRESS()),
-            Errors::already_published(EACCOUNT_OPERATIONS_CAPABILITY)
-        );
-        move_to(
-            lr_account,
-            AccountOperationsCapability {
-                limits_cap: AccountLimits::grant_mutation_capability(lr_account),
-            }
-        );
-
-        assert(
-            !exists<LibraWriteSetManager>(CoreAddresses::LIBRA_ROOT_ADDRESS()),
-            Errors::already_published(EWRITESET_MANAGER)
-        );
-        move_to(
-            lr_account,
-            LibraWriteSetManager {
-                upgrade_events: Event::new_event_handle<Self::UpgradeEvent>(lr_account),
-            }
-        );


I feels odd for this to be in the in the create_libra_root_account function. What would you think of doing something like:

create_libra_root_account(...); // As before emit_event<CreateAccountEvent>(....); // <- emit create account event for LR root account. <THIS CODE> create_treasury_compliance_account(...);

here?

Unfortunately, this won't wok because emitting the event (which happens in create_libra_root_account) requires <THIS_CODE> to be run first, specifically, the AccountOperationsCapability part.

I actually felt good about moving this code into create_libra_root_account because the pattern we use elsewhere is to publish X role-specific resources in create_X_account. But certainly open to alternatives!

ahhh I see! For some reason I didn't see that the event emission is happening in make_account.

I'm fine with this as-is since I can't think of any other alternatives that wouldn't make the code more complicated.

tzakian · 2020-09-23T15:50:41Z

language/stdlib/modules/LibraAccount.move

+    /// Message for creation of a new account
+    struct CreateAccountEvent {
+        /// Address of the account that was created
+        created: address,


What would you think of also including the role id for the account created in this event?

I love it except that role id's will no longer be opaque. But I'm ok with that because they can already be read off-chain...

tzakian · 2020-09-23T17:16:33Z

language/tools/vm-genesis/src/lib.rs

+        .iter()
+        .find(|e| e.key() == &NewEpochEvent::event_key())
+        .expect("Couldn't find NewEpochEvent");
+    assert_eq!(*new_epoch_event.key(), NewEpochEvent::event_key());


What would you think of adding an additional check that there's only one new epoch event?

Very good idea, would not want to make that mistake (and it'd be easy to make!).

tzakian

LGTM!

MIRAI-bot · 2020-09-23T19:06:23Z

[move-prover] Move Prover test (language/move-prover/cargo test) on commit 8877d8b did fail. See details.

sblackshear · 2020-09-24T14:43:29Z

/canary

- New CreateAccountEvent struct that records the created address - Had to refactor LibraAccount::initialize() a bit to ensure that we can emit an event for the creation of LibraRoot - Had to refactor the event verification part of VM genesis since the number of events now depends on the # of created accounts - Added tests for each of the create_*_account functions - Added Rust wrapper for CreateAccountEvent Previously, users saw only TYPE_MISMATCH, NUMBER_OF_TYPE_ARGUMENTS_MISMATCH, or LINKER_ERROR, which are not very descriptive Closes: #6118 Closes: #6156

github-actions · 2020-09-24T15:07:57Z

Cluster Test Result

Cluster setup failed: `Nodes did not become healthy after deployment`
Logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-24T14:56:15Z',to:'2020-09-24T15:07:56Z'))
Dashboard: http://grafana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/d/2XqUIhnWz/performance?from=1600959375000&to=1600960076000
Validator 1 logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-24T14:56:15Z',to:'2020-09-24T15:07:56Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

❗ Cluster Test failed - non-zero exit code for cti

Repro cmd:

./scripts/cti --tag land_441d1e4b --cluster-test-tag land_84eb8678 -E RUST_LOG=debug -E BATCH_SIZE=15 -E UPDATE_TO_TAG=land_84eb8678 --report report.json --suite land_blocking_compat

bors-libra · 2020-09-24T15:08:02Z

💔 Test Failed - Build images and run cluster test

sblackshear · 2020-09-24T17:17:57Z

/canary

- New CreateAccountEvent struct that records the created address - Had to refactor LibraAccount::initialize() a bit to ensure that we can emit an event for the creation of LibraRoot - Had to refactor the event verification part of VM genesis since the number of events now depends on the # of created accounts - Added tests for each of the create_*_account functions - Added Rust wrapper for CreateAccountEvent Previously, users saw only TYPE_MISMATCH, NUMBER_OF_TYPE_ARGUMENTS_MISMATCH, or LINKER_ERROR, which are not very descriptive Closes: #6118 Closes: #6156

github-actions · 2020-09-24T17:48:44Z

Cluster Test Result

Cluster setup failed: `Nodes did not become healthy after deployment`
Logs: http://kibana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-24T17:31:21Z',to:'2020-09-24T17:48:43Z'))
Dashboard: http://grafana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/d/2XqUIhnWz/performance?from=1600968681000&to=1600969723000
Validator 1 logs: http://kibana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-24T17:31:21Z',to:'2020-09-24T17:48:43Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

❗ Cluster Test failed - non-zero exit code for cti

Repro cmd:

./scripts/cti --tag land_a8205421 --cluster-test-tag land_9e51266c -E RUST_LOG=debug -E BATCH_SIZE=15 -E UPDATE_TO_TAG=land_9e51266c --report report.json --suite land_blocking_compat

bors-libra · 2020-09-24T17:48:50Z

💔 Test Failed - Build images and run cluster test

sblackshear · 2020-09-24T20:20:05Z

/canary

- New CreateAccountEvent struct that records the created address - Had to refactor LibraAccount::initialize() a bit to ensure that we can emit an event for the creation of LibraRoot - Had to refactor the event verification part of VM genesis since the number of events now depends on the # of created accounts - Added tests for each of the create_*_account functions - Added Rust wrapper for CreateAccountEvent Previously, users saw only TYPE_MISMATCH, NUMBER_OF_TYPE_ARGUMENTS_MISMATCH, or LINKER_ERROR, which are not very descriptive Closes: #6118 Closes: #6156

sblackshear · 2020-09-25T02:47:34Z

Ahh I forgot about that. Thanks!

sblackshear · 2020-09-25T02:48:51Z

/canary

- New CreateAccountEvent struct that records the created address - Had to refactor LibraAccount::initialize() a bit to ensure that we can emit an event for the creation of LibraRoot - Had to refactor the event verification part of VM genesis since the number of events now depends on the # of created accounts - Added tests for each of the create_*_account functions - Added Rust wrapper for CreateAccountEvent Previously, users saw only TYPE_MISMATCH, NUMBER_OF_TYPE_ARGUMENTS_MISMATCH, or LINKER_ERROR, which are not very descriptive Closes: #6118 Closes: #6156

github-actions · 2020-09-25T03:13:00Z

Cluster Test Result

all up : 1008 TPS, 4485 ms latency, 5350 ms p99 latency, no expired txns
Logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-25T03:01:28Z',to:'2020-09-25T03:12:59Z'))
Dashboard: http://grafana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/d/2XqUIhnWz/performance?from=1601002888000&to=1601003579000
Validator 1 logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-25T03:01:28Z',to:'2020-09-25T03:12:59Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

./scripts/cti --tag land_2b06201d -E RUST_LOG=debug --report report.json --suite land_blocking

🎉 Land-blocking cluster test passed! 👌

bors-libra · 2020-09-25T03:18:34Z

☀️ Canary successful

sblackshear · 2020-09-25T03:26:00Z

/land

bors-libra · 2020-09-25T05:46:15Z

🔒 Merge Conflict

sblackshear · 2020-09-25T21:46:17Z

/land

MIRAI-bot · 2020-09-25T22:06:20Z

[move-prover] Move Prover test (language/move-prover/cargo test) on commit d2c6376 did fail. See details.

sblackshear · 2020-09-25T22:10:57Z

/cancel

sblackshear · 2020-09-26T01:26:31Z

/land

MIRAI-bot · 2020-09-26T01:32:53Z

[move-prover] Move Prover test (language/move-prover/cargo test) on commit 437f9d7 did fail. See details.

- New CreateAccountEvent struct that records the created address - Had to refactor LibraAccount::initialize() a bit to ensure that we can emit an event for the creation of LibraRoot - Had to refactor the event verification part of VM genesis since the number of events now depends on the # of created accounts - Added tests for each of the create_*_account functions - Added Rust wrapper for CreateAccountEvent Previously, users saw only TYPE_MISMATCH, NUMBER_OF_TYPE_ARGUMENTS_MISMATCH, or LINKER_ERROR, which are not very descriptive Closes: diem#6118 Closes: diem#6156

github-actions · 2020-09-26T06:04:43Z

Cluster Test Result

all up : 1001 TPS, 4519 ms latency, 5500 ms p99 latency, no expired txns
Logs: http://kibana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-26T05:48:16Z',to:'2020-09-26T06:04:42Z'))
Dashboard: http://grafana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/d/2XqUIhnWz/performance?from=1601099296000&to=1601100282000
Validator 1 logs: http://kibana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-09-26T05:48:16Z',to:'2020-09-26T06:04:42Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

./scripts/cti --tag land_6726e579 -E RUST_LOG=debug --report report.json --suite land_blocking

🎉 Land-blocking cluster test passed! 👌

sblackshear requested review from tzakian, dmitri-perelman and moeziniaf September 23, 2020 05:26

bors-libra added this to In Review in bors Sep 23, 2020

diem-cla-bot bot added the cla-signed label Sep 23, 2020

sblackshear force-pushed the create_account_event branch from c25e3f1 to dda7b3e Compare September 23, 2020 14:08

tzakian reviewed Sep 23, 2020

View reviewed changes

tzakian approved these changes Sep 23, 2020

View reviewed changes

sblackshear force-pushed the create_account_event branch from dda7b3e to 8877d8b Compare September 23, 2020 18:49

sblackshear force-pushed the create_account_event branch 3 times, most recently from 2b860d2 to 24cb838 Compare September 24, 2020 02:52

bob-wilson mentioned this pull request Sep 24, 2020

Stdlib cleanup: Audit events to ensure that we have all the events we need + do not have superfluous events [P2] #4274

Closed

bors-libra moved this from In Review to Canary in bors Sep 24, 2020

bors-libra moved this from Canary to In Review in bors Sep 24, 2020

bors-libra moved this from In Review to Canary in bors Sep 24, 2020

bors-libra moved this from Canary to In Review in bors Sep 24, 2020

sblackshear force-pushed the create_account_event branch from 24cb838 to 119cfba Compare September 24, 2020 20:19

bors-libra moved this from In Review to Canary in bors Sep 24, 2020

bors-libra moved this from In Review to Canary in bors Sep 25, 2020

bors-libra moved this from Canary to In Review in bors Sep 25, 2020

bors-libra moved this from In Review to Queued in bors Sep 25, 2020

bors-libra moved this from Queued to In Review in bors Sep 25, 2020

sblackshear force-pushed the create_account_event branch from 71b0e07 to d2c6376 Compare September 25, 2020 21:45

bors-libra moved this from In Review to Queued in bors Sep 25, 2020

bors-libra moved this from Queued to In Review in bors Sep 25, 2020

sblackshear force-pushed the create_account_event branch 2 times, most recently from 437f9d7 to b49195c Compare September 26, 2020 01:20

bors-libra moved this from In Review to Queued in bors Sep 26, 2020

bors-libra moved this from Queued to Testing in bors Sep 26, 2020

bors-libra force-pushed the create_account_event branch from b49195c to 6726e57 Compare September 26, 2020 06:11

bors-libra removed this from Testing in bors Sep 26, 2020

bors-libra closed this in 6726e57 Sep 26, 2020

bors-libra merged commit 6726e57 into diem:master Sep 26, 2020

xli mentioned this pull request Sep 30, 2020

[json-rpc] add more tests #6280

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[breaking][libra framework] add event on account creation #6156

[breaking][libra framework] add event on account creation #6156

sblackshear commented Sep 23, 2020

tzakian Sep 23, 2020 •

edited

sblackshear Sep 23, 2020

tzakian Sep 23, 2020

tzakian Sep 23, 2020

sblackshear Sep 23, 2020

tzakian Sep 23, 2020

sblackshear Sep 23, 2020

tzakian left a comment

MIRAI-bot commented Sep 23, 2020

sblackshear commented Sep 24, 2020

github-actions bot commented Sep 24, 2020

bors-libra commented Sep 24, 2020

sblackshear commented Sep 24, 2020

github-actions bot commented Sep 24, 2020

bors-libra commented Sep 24, 2020

sblackshear commented Sep 24, 2020

sblackshear commented Sep 25, 2020

sblackshear commented Sep 25, 2020

github-actions bot commented Sep 25, 2020

bors-libra commented Sep 25, 2020

sblackshear commented Sep 25, 2020

bors-libra commented Sep 25, 2020

sblackshear commented Sep 25, 2020

MIRAI-bot commented Sep 25, 2020

sblackshear commented Sep 25, 2020

sblackshear commented Sep 26, 2020

MIRAI-bot commented Sep 26, 2020

github-actions bot commented Sep 26, 2020

[breaking][libra framework] add event on account creation #6156

[breaking][libra framework] add event on account creation #6156

Conversation

sblackshear commented Sep 23, 2020

tzakian Sep 23, 2020 • edited

Choose a reason for hiding this comment

sblackshear Sep 23, 2020

Choose a reason for hiding this comment

tzakian Sep 23, 2020

Choose a reason for hiding this comment

tzakian Sep 23, 2020

Choose a reason for hiding this comment

sblackshear Sep 23, 2020

Choose a reason for hiding this comment

tzakian Sep 23, 2020

Choose a reason for hiding this comment

sblackshear Sep 23, 2020

Choose a reason for hiding this comment

tzakian left a comment

Choose a reason for hiding this comment

MIRAI-bot commented Sep 23, 2020

sblackshear commented Sep 24, 2020

github-actions bot commented Sep 24, 2020

bors-libra commented Sep 24, 2020

sblackshear commented Sep 24, 2020

github-actions bot commented Sep 24, 2020

bors-libra commented Sep 24, 2020

sblackshear commented Sep 24, 2020

sblackshear commented Sep 25, 2020

sblackshear commented Sep 25, 2020

github-actions bot commented Sep 25, 2020

bors-libra commented Sep 25, 2020

sblackshear commented Sep 25, 2020

bors-libra commented Sep 25, 2020

sblackshear commented Sep 25, 2020

MIRAI-bot commented Sep 25, 2020

sblackshear commented Sep 25, 2020

sblackshear commented Sep 26, 2020

MIRAI-bot commented Sep 26, 2020

github-actions bot commented Sep 26, 2020

tzakian Sep 23, 2020 •

edited