da stake table test #3716
da stake table test #3716
Conversation
pls148
requested review from
bfish713,
imabdulbasit,
jbearer,
lukaszrzasik,
rob-maron,
ss-es and
sveitser
as code owners
pls148
force-pushed
the
ps/da-stake-table-test
branch
from
034eaca to
4175c2e
Compare
|
Unable to build without Cargo.lock file. This means that after this change 3rd party projects may have For the first failing build see: https://github.com/EspressoSystems/espresso-network/actions/runs/19083829552 To reproduce locally run This PR can still be merged. |
pls148
force-pushed
the
ps/da-stake-table-test
branch
from
9773d05 to
82efb5f
Compare
pls148
force-pushed
the
ps/da-stake-table-test
branch
from
82efb5f to
96420cd
Compare
pls148
force-pushed
the
ps/da-stake-table-test
branch
from
559e058 to
b6b3d45
Compare
pls148
force-pushed
the
ps/da-stake-table-test
branch
from
b6b3d45 to
855fb91
Compare
ss-es
left a comment
ss-es
left a comment
There was a problem hiding this comment.
some questions before approving, but on the whole nothing major
| ] | ||
|
|
||
| # 0, 2, 4 - change 33% | ||
| [[da_committees]] |
There was a problem hiding this comment.
maybe @bfish713 can comment on which of these are most realistic
I'm sort of inclined to say we don't need to run all of these in CI, or maybe we can split this off into 3 separate tests which is closer to what we'd like to do anyway (i.e. one da committee change per upgrade)? but it's not a strong opinion
There was a problem hiding this comment.
11 minutes for a CI job is also more than fine anyway
tests/proof_of_stake.rs
Outdated
| let entries: &[PeerConfig<SeqTypes>] = &[PeerConfig { | ||
| stake_table_entry: make_stake_table_entry("BLS_VER_KEY~bQszS-QKYvUij2g20VqS8asttGSb95NrTu2PUj0uMh1CBUxNy1FqyPDjZqB29M7ZbjWqj79QkEOWkpga84AmDYUeTuWmy-0P1AdKHD3ehc-dKvei78BDj5USwXPJiDUlCxvYs_9rWYhagaq-5_LXENr78xel17spftNd5MA1Mw5U"), | ||
| state_ver_key: make_state_ver_key("SCHNORR_VER_KEY~ibJCbfPOhDoURqiGLe683TDJ_KOLQCx8_Hdq43dOviSuL6WJJ_2mARKO3xA2k5zpXE3iiq4_z7mzvA-V1VXvIWw"), | ||
| },PeerConfig { | ||
| stake_table_entry:make_stake_table_entry("BLS_VER_KEY~4zQnaCOFJ7m95OjxeNls0QOOwWbz4rfxaL3NwmN2zSdnf8t5Nw_dfmMHq05ee8jCegw6Bn5T8inmrnGGAsQJMMWLv77nd7FJziz2ViAbXg-XGGF7o4HyzELCmypDOIYF3X2UWferFE_n72ZX0iQkUhOvYZZ7cfXToXxRTtb_mwRR"), | ||
| state_ver_key: make_state_ver_key("SCHNORR_VER_KEY~lNCMqH5qLthH5OXxW_Z25tLXJUqmzzhsuQ6oVuaPWhtRPmgIKSqcBoJTaEbmGZL2VfTyQNguaoQL4U_4tCA_HmI"), | ||
| },PeerConfig { | ||
| stake_table_entry: make_stake_table_entry("BLS_VER_KEY~IBRoz_Q1EXvcm1pNZcmVlyYZU8hZ7qmy337ePAjEMhz8Hl2q8vWPFOd3BaLwgRS1UzAPW3z4E-XIgRDGcRBTAMZX9b_0lKYjlyTlNF2EZfNnKmvv-xJ0yurkfjiveeYEsD2l5d8q_rJJbH1iZdXy-yPEbwI0SIvQfwdlcaKw9po4"), | ||
| state_ver_key: make_state_ver_key("SCHNORR_VER_KEY~nkFKzpLhJAafJ3LBkY_0h9OzxSyTu95Z029EUFPO4QNkeUo6DHQGTTVjxmprTA5H8jRSn73i0slJvig6dZ5kLX4"), | ||
| },PeerConfig { | ||
| stake_table_entry: make_stake_table_entry("BLS_VER_KEY~rO2PIjyY30HGfapFcloFe3mNDKMIFi6JlOLkH5ZWBSYoRm5fE2-Rm6Lp3EvmAcB5r7KFJ0c1Uor308x78r04EY_sfjcsDCWt7RSJdL4cJoD_4fSTCv_bisO8k98hs_8BtqQt8BHlPeJohpUXvcfnK8suXJETiJ6Er97pfxRbzgAL"), | ||
| state_ver_key: make_state_ver_key("SCHNORR_VER_KEY~NwYhzlWarlZHxTNvChWuf74O3fP7zIt5NdC7V8gV6w2W92JOBDkrNmKQeMGxMUke-G5HHxUjHlZEWr1m1xLjEaI"), | ||
| },PeerConfig { | ||
| stake_table_entry: make_stake_table_entry("BLS_VER_KEY~r6b-Cwzp-b3czlt0MHmYPJIow5kMsXbrNmZsLSYg9RV49oCCO4WEeCRFR02x9bqLCa_sgNFMrIeNdEa11qNiBAohApYFIvrSa-zP5QGj3xbZaMOCrshxYit6E2TR-XsWvv6gjOrypmugjyTAth-iqQzTboSfmO9DD1-gjJIdCaD7"), | ||
| state_ver_key: make_state_ver_key("SCHNORR_VER_KEY~qMfMj1c1hRVTnugvz3MKNnVC5JA9jvZcV3ZCLL_J4Ap-u0i6ulGWveTk3OOelZj2-kd_WD5ojtYGWV1jHx9wCaA"), | ||
| }]; |
There was a problem hiding this comment.
rather than hard-coding this here would it make more sense to take it from genesis that was loaded earlier? because really what we want to check is that the queried da committee matches what we set it to in the genesis file
tests/proof_of_stake.rs
Outdated
| ) | ||
| .await?; | ||
|
|
||
| let epoch_length = genesis.epoch_height.expect("epoch_height set in genesis"); |
There was a problem hiding this comment.
should the log say "epoch height not set in genesis"?
tests/proof_of_stake.rs
Outdated
| assert_eq!( | ||
| da_stake_table.stake_table.len(), | ||
| entries.len(), | ||
| "expected lengths of da stake tables to match. Expected: {entries:?}, Got: {:?}", | ||
| da_stake_table.stake_table | ||
| ); | ||
|
|
||
| for entry in entries { | ||
| assert!( | ||
| da_stake_table.stake_table.iter().any(|e| entry == &e), | ||
| "DA stake table missing expected entry: {entry:?}. Expected entries: {entries:?}, \ | ||
| Got: {:?}", | ||
| da_stake_table.stake_table | ||
| ); | ||
| } |
There was a problem hiding this comment.
this seems a bit odd, can't we compare the stake tables with == directly? (maybe casting to a sorted vec or something)
Closes #<ISSUE_NUMBER>
This PR:
This PR does not:
Key places to review: