[diem-framework] Implement CRSNs in the Diem Framework

[tzakian]

This PR implements CRSNs as defined in DIP-168, along with the transactions to opt-in to CRSNs, and force expire CRSNs.

This will need to be plumbed into the account prologue (CRSN::check) and epilogue (CRSN::record). But that will be done later in a separate PR.

Testing

Added a number of Move unit tests to the CRSN module

[sblackshear]

Hyperlink to the DIP could be useful (once it lands)

[sblackshear]

Looks pretty excellent to me! One related question: what is the migration plan for SlidingNonce?

[sblackshear]

I think we can still call BitVector methods without the Self important (but could be wrong)

[tzakian]

Still need the Self import in order to call BitVector functions -- this is similar to self imports in Rust.

[sblackshear]

File name looks wrong here--checked in by mistake?

[tzakian]

Woops...I'll remove

[tzakian]

What is the migration plan for SlidingNonce?

Good question. My thinking is:

1. AOS publishes the CRSN, keep using sliding nonce and sequence numbers as they today (this should work fine).
2. Switch over to using sequence nonces, and using those (so no longer using sequence numbers). Freeze all sliding nonce numbers sent to existing transaction scripts at 0 (sliding nonce does no checking/recording in this case).

Later on, we can update all of the scripts, and remove the sliding nonce module/remove the resource, but I think that would be better to be done separately from the switchover, otherwise step 2 will also involve switching to an entirely new set of scripts simultaneously.

What are your thoughts on this migration strategy?

[sblackshear]

What are your thoughts on this migration strategy?

Yep, that sounds like a good plan to me! I was initially thinking that trying to combine the code might make sense, but I think having both + deprecating is much more sensible.

[tzakian]

/land

[github-actions]

Cluster Test Result

Test runner setup time spent 273 secs
Compatibility test results for land_280dd3b2 ==> land_765444da (PR)
1. All instances running land_280dd3b2, generating some traffic on network
2. First full node land_280dd3b2 ==> land_765444da, to validate new full node to old validator node traffic
3. First Validator node land_280dd3b2 ==> land_765444da, to validate storage compatibility
4. First batch validators (14) land_280dd3b2 ==> land_765444da, to test consensus and traffic between old full nodes and new validator node
5. First batch full nodes (14) land_280dd3b2 ==> land_765444da
6. Second batch validators (15) land_280dd3b2 ==> land_765444da, to upgrade rest of the validators
7. Second batch of full nodes (15) land_280dd3b2 ==> land_765444da, to finish the network upgrade, time spent 715 secs
all up : 890 TPS, 5115 ms latency, 6000 ms p99 latency, no expired txns, time spent 251 secs
Logs: http://kibana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-05-19T00:33:24Z',to:'2021-05-19T00:57:25Z'))
Dashboard: http://grafana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/d/performance/performance?from=1621384404000&to=1621385845000
Validator 1 logs: http://kibana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-05-19T00:33:24Z',to:'2021-05-19T00:57:25Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

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

🎉 Land-blocking cluster test passed! 👌

[bors-libra]

💔 Test Failed - ci-test

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[bob-wilson]

@tzakian We should reopen this, right? (I'm not sure if it might have been copied to a different version of the PR or I would just do it myself.)

[tzakian]

@tzakian We should reopen this, right? (I'm not sure if it might have been copied to a different version of the PR or I would just do it myself.)

@bob-wilson Good question. This PR has been rolled into #8528 so I'm planning on landing (/would prefer to land) these changes in that PR since there were some additional changes needed to make things work with mempool/prologue/epilogue that only work in context with the other changes. So I think keeping this PR closed makes sense.