-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(consensus): add L2 contracts VM storage reader (BFT-434) #2416
Changes from all commits
e9b7c36
5a1a259
a8779b5
477ea5e
5a925c6
f7b1b9b
5680805
42a974c
2ad2c32
899f07b
d4e6f84
cbc46a1
f1cda3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
[submodule "contracts"] | ||
path = contracts | ||
url = https://github.com/matter-labs/era-contracts.git | ||
url = https://github.com/matter-labs/era-contracts | ||
branch = consensus_contracts | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
use ethabi::Contract; | ||
|
||
use crate::{load_contract, read_bytecode, TestContract}; | ||
|
||
const CONSENSUS_REGISTRY_CONTRACT_FILE: &str = | ||
"contracts/l2-contracts/artifacts-zk/contracts/ConsensusRegistry.sol/ConsensusRegistry.json"; | ||
|
||
pub fn load_consensus_registry_contract() -> Contract { | ||
load_contract(CONSENSUS_REGISTRY_CONTRACT_FILE) | ||
} | ||
|
||
pub fn load_consensus_registry_contract_in_test() -> TestContract { | ||
TestContract { | ||
bytecode: read_bytecode(CONSENSUS_REGISTRY_CONTRACT_FILE), | ||
contract: load_consensus_registry_contract(), | ||
factory_deps: vec![], | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,24 @@ | ||
//! Storage test helpers. | ||
|
||
use anyhow::Context as _; | ||
use zksync_concurrency::{ctx, error::Wrap as _, time}; | ||
use zksync_concurrency::{ctx, ctx::Ctx, error::Wrap as _, time}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: don't import ctx::Ctx, use the qualified name. |
||
use zksync_consensus_roles::validator; | ||
use zksync_contracts::BaseSystemContracts; | ||
use zksync_contracts::{consensus_l2_contracts, BaseSystemContracts}; | ||
use zksync_dal::CoreDal as _; | ||
use zksync_node_genesis::{insert_genesis_batch, mock_genesis_config, GenesisParams}; | ||
use zksync_node_test_utils::{recover, snapshot, Snapshot}; | ||
use zksync_state_keeper::testonly::fee; | ||
use zksync_test_account::{Account, TxType}; | ||
use zksync_types::{ | ||
commitment::L1BatchWithMetadata, protocol_version::ProtocolSemanticVersion, | ||
system_contracts::get_system_smart_contracts, L1BatchNumber, L2BlockNumber, ProtocolVersionId, | ||
commitment::L1BatchWithMetadata, | ||
ethabi::{Address, Contract, Token}, | ||
protocol_version::ProtocolSemanticVersion, | ||
system_contracts::get_system_smart_contracts, | ||
Execute, L1BatchNumber, L2BlockNumber, ProtocolVersionId, Transaction, | ||
}; | ||
|
||
use super::ConnectionPool; | ||
use crate::testonly::StateKeeper; | ||
|
||
pub(crate) fn mock_genesis_params(protocol_version: ProtocolVersionId) -> GenesisParams { | ||
let mut cfg = mock_genesis_config(); | ||
|
@@ -189,3 +195,111 @@ impl ConnectionPool { | |
Ok(()) | ||
} | ||
} | ||
|
||
/// A struct for writing to consensus L2 contracts. | ||
#[derive(Debug)] | ||
pub struct VMWriter { | ||
pool: ConnectionPool, | ||
node: StateKeeper, | ||
account: Account, | ||
} | ||
|
||
impl VMWriter { | ||
/// Constructs a new `VMWriter` instance. | ||
pub fn new(pool: ConnectionPool, node: StateKeeper, account: Account) -> Self { | ||
Self { | ||
pool, | ||
node, | ||
account, | ||
} | ||
} | ||
|
||
/// Deploys the consensus registry contract and adds nodes to it. | ||
pub async fn deploy_and_add_nodes( | ||
&mut self, | ||
ctx: &Ctx, | ||
owner: Address, | ||
nodes: &[&[Token]], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can the conversion be done inside the function, passing &[&[Token]] is very loosely typed. |
||
) -> Address { | ||
let registry_contract = consensus_l2_contracts::load_consensus_registry_contract_in_test(); | ||
|
||
let mut txs: Vec<Transaction> = vec![]; | ||
let deploy_tx = self.account.get_deploy_tx_with_factory_deps( | ||
®istry_contract.bytecode, | ||
Some(&[Token::Address(owner)]), | ||
vec![], | ||
TxType::L2, | ||
); | ||
txs.push(deploy_tx.tx); | ||
for node in nodes { | ||
let tx = self.gen_tx_add(®istry_contract.contract, deploy_tx.address, node); | ||
txs.push(tx); | ||
} | ||
txs.push( | ||
self.gen_tx_set_validator_committee(deploy_tx.address, ®istry_contract.contract), | ||
Comment on lines
+235
to
+239
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any rason why the |
||
); | ||
txs.push( | ||
self.gen_tx_set_attester_committee(deploy_tx.address, ®istry_contract.contract), | ||
); | ||
|
||
self.node.push_block(&txs).await; | ||
self.pool | ||
.wait_for_payload(ctx, self.node.last_block()) | ||
.await | ||
.unwrap(); | ||
|
||
deploy_tx.address | ||
} | ||
|
||
fn gen_tx_add( | ||
&mut self, | ||
contract: &Contract, | ||
contract_address: Address, | ||
input: &[Token], | ||
) -> Transaction { | ||
let calldata = contract | ||
.function("add") | ||
.unwrap() | ||
.encode_input(input) | ||
.unwrap(); | ||
self.gen_tx(contract_address, calldata) | ||
} | ||
|
||
fn gen_tx_set_validator_committee( | ||
&mut self, | ||
contract_address: Address, | ||
contract: &Contract, | ||
) -> Transaction { | ||
let calldata = contract | ||
.function("setValidatorCommittee") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see this method in matter-labs/era-contracts#555 ; is it commitValidators ? |
||
.unwrap() | ||
.short_signature() | ||
.to_vec(); | ||
self.gen_tx(contract_address, calldata) | ||
} | ||
|
||
fn gen_tx_set_attester_committee( | ||
&mut self, | ||
contract_address: Address, | ||
contract: &Contract, | ||
) -> Transaction { | ||
let calldata = contract | ||
.function("setAttesterCommittee") | ||
.unwrap() | ||
.short_signature() | ||
.to_vec(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all these methods wrapping raw contract API deserve a dedicated module. |
||
self.gen_tx(contract_address, calldata) | ||
} | ||
|
||
fn gen_tx(&mut self, contract_address: Address, calldata: Vec<u8>) -> Transaction { | ||
self.account.get_l2_tx_for_execute( | ||
Execute { | ||
contract_address, | ||
calldata, | ||
value: Default::default(), | ||
factory_deps: vec![], | ||
}, | ||
Some(fee(10_000_000)), | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
use rand::Rng; | ||
use zksync_basic_types::web3::contract::Tokenize; | ||
use zksync_concurrency::{ctx, scope}; | ||
use zksync_types::{ | ||
api::{BlockId, BlockNumber}, | ||
ethabi::{Address, Token}, | ||
L2ChainId, ProtocolVersionId, U256, | ||
}; | ||
|
||
use crate::storage::ConnectionPool; | ||
|
||
#[tokio::test(flavor = "multi_thread")] | ||
async fn test_vm_reader() { | ||
zksync_concurrency::testonly::abort_on_panic(); | ||
let ctx = &ctx::test_root(&ctx::RealClock); | ||
let rng = &mut ctx.rng(); | ||
|
||
scope::run!(ctx, |ctx, s| async { | ||
let pool = ConnectionPool::test(false, ProtocolVersionId::latest()).await; | ||
let (node, runner) = crate::testonly::StateKeeper::new(ctx, pool.clone()).await?; | ||
let account = runner.account.clone(); | ||
s.spawn_bg(runner.run_real(ctx)); | ||
|
||
let mut writer = super::testonly::VMWriter::new(pool.clone(), node, account.clone()); | ||
|
||
let mut nodes: Vec<Vec<Token>> = Vec::new(); | ||
let num_nodes = 5; | ||
for _ in 0..num_nodes { | ||
let node_entry = ( | ||
Address::random(), | ||
U256::from(rng.gen::<usize>()), | ||
(0..256).map(|_| rng.gen()).collect::<Vec<u8>>(), | ||
(0..256).map(|_| rng.gen()).collect::<Vec<u8>>(), | ||
U256::from(rng.gen::<usize>()), | ||
(0..256).map(|_| rng.gen()).collect::<Vec<u8>>(), | ||
) | ||
.into_tokens(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you could use abigen! to generate bindings for all the Solidity structs and then you get the tokenization with static typing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't migrate to |
||
nodes.push(node_entry); | ||
} | ||
let nodes_ref: Vec<&[Token]> = nodes.iter().map(|v| v.as_slice()).collect(); | ||
let nodes_slice: &[&[Token]] = nodes_ref.as_slice(); | ||
let registry_address = writer | ||
.deploy_and_add_nodes(ctx, account.address, nodes_slice) | ||
.await; | ||
|
||
let (tx_sender, _) = zksync_node_api_server::web3::testonly::create_test_tx_sender( | ||
pool.0.clone(), | ||
L2ChainId::default(), | ||
zksync_node_api_server::execution_sandbox::TransactionExecutor::Real, | ||
) | ||
.await; | ||
let block_id = BlockId::Number(BlockNumber::Pending); | ||
let reader = | ||
super::vm_reader::VMReader::new(pool.clone(), tx_sender.clone(), registry_address); | ||
|
||
let (validators, attesters) = reader.read_committees(ctx, block_id).await.unwrap(); | ||
assert_eq!(validators.len(), num_nodes); | ||
assert_eq!(attesters.len(), num_nodes); | ||
for i in 0..nodes.len() { | ||
assert_eq!( | ||
nodes[i][0].clone().into_address().unwrap(), | ||
validators[i].node_owner | ||
); | ||
assert_eq!( | ||
nodes[i][1].clone().into_uint().unwrap().as_usize(), | ||
validators[i].weight | ||
); | ||
assert_eq!( | ||
nodes[i][2].clone().into_bytes().unwrap(), | ||
validators[i].pub_key | ||
); | ||
assert_eq!(nodes[i][3].clone().into_bytes().unwrap(), validators[i].pop); | ||
|
||
assert_eq!( | ||
nodes[i][0].clone().into_address().unwrap(), | ||
attesters[i].node_owner | ||
); | ||
assert_eq!( | ||
nodes[i][4].clone().into_uint().unwrap().as_usize(), | ||
attesters[i].weight | ||
); | ||
assert_eq!( | ||
nodes[i][5].clone().into_bytes().unwrap(), | ||
attesters[i].pub_key | ||
); | ||
} | ||
|
||
Ok(()) | ||
}) | ||
.await | ||
.unwrap(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JFYI: I don't think the PR is mergeable until we can merge the contracts to main (e.g. we cannot live with a separate branch in submodules).
I think we should start a discussion with the protocol team on how to merge these changes to
main
without having to finish/audit them, and meanwhile minimize risks of them somehow affecting production environments.I have a few ideas, let's discuss in Slack. cc @RomanBrodetski @brunoffranca