Skip to content

Commit 5895f51

Browse files
security: data size checks (#219)
Co-authored-by: jotabulacios <jbulacios@fi.uba.ar>
1 parent fc8b45c commit 5895f51

9 files changed

Lines changed: 73 additions & 12 deletions

File tree

batcher/src/config/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ pub struct ECDSAConfig {
1010
pub struct BatcherConfigFromYaml {
1111
pub block_interval: u64,
1212
pub batch_size_interval: usize,
13+
pub max_proof_size: usize,
14+
pub max_batch_size: usize,
1315
}
1416

1517
#[derive(Debug, Deserialize)]

batcher/src/lib.rs

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ pub struct Batcher {
3636
current_batch: Mutex<Vec<VerificationData>>,
3737
max_block_interval: u64,
3838
min_batch_size: usize,
39+
max_proof_size: usize,
40+
max_batch_size: usize,
3941
last_uploaded_batch_block: Mutex<u64>,
4042
}
4143

@@ -78,6 +80,8 @@ impl Batcher {
7880
current_batch: Mutex::new(Vec::new()),
7981
max_block_interval: config.batcher.block_interval,
8082
min_batch_size: config.batcher.batch_size_interval,
83+
max_proof_size: config.batcher.max_proof_size,
84+
max_batch_size: config.batcher.max_batch_size,
8185
last_uploaded_batch_block: Mutex::new(last_uploaded_batch_block),
8286
}
8387
}
@@ -155,7 +159,7 @@ impl Batcher {
155159
serde_json::from_str(message.to_text().expect("Message is not text"))
156160
.expect("Failed to deserialize task");
157161

158-
if verification_data.verify() {
162+
if verification_data.proof.len() <= self.max_proof_size && verification_data.verify() {
159163
self.add_to_batch(verification_data).await;
160164
} else {
161165
// FIXME(marian): Handle this error correctly
@@ -207,14 +211,43 @@ impl Batcher {
207211

208212
async fn process_batch_and_update_state(&self, block_number: u64) -> (Vec<u8>, [u8; 32]) {
209213
let mut current_batch = self.current_batch.lock().await;
210-
let batch_commitment = VerificationCommitmentBatch::from(&(*current_batch));
214+
215+
216+
let mut batch_bytes =
217+
serde_json::to_vec(current_batch.as_slice()).expect("Failed to serialize batch");
218+
219+
let batch_to_send;
220+
if batch_bytes.len() > self.max_batch_size {
221+
let mut current_batch_end = 0; // not inclusive
222+
let mut current_batch_size = 0;
223+
for (i, verification_data) in current_batch.iter().enumerate() {
224+
let verification_data_bytes = serde_json::to_vec(verification_data)
225+
.expect("Failed to serialize verification data");
226+
227+
current_batch_size += verification_data_bytes.len();
228+
if current_batch_size > self.max_batch_size {
229+
current_batch_end = i;
230+
break;
231+
}
232+
}
233+
234+
debug!("Batch size exceeds max batch size, splitting batch at index: {}", current_batch_end);
235+
batch_to_send = current_batch.drain(..current_batch_end)
236+
.collect::<Vec<_>>();
237+
238+
info!("# of Elements remaining for next batch: {}", current_batch.len());
239+
batch_bytes = serde_json::to_vec(&batch_to_send)
240+
.expect("Failed to serialize batch");
241+
} else {
242+
batch_to_send = current_batch.clone();
243+
current_batch.clear();
244+
}
245+
246+
let batch_commitment = VerificationCommitmentBatch::from(&batch_to_send);
211247
let batch_merkle_tree: MerkleTree<VerificationCommitmentBatch> =
212248
MerkleTree::build(&batch_commitment.0);
213-
let batch_bytes =
214-
serde_json::to_vec(current_batch.as_slice()).expect("Failed to serialize batch");
215249

216-
// update batcher state (clear current batch and update last uploaded batch block)
217-
current_batch.clear();
250+
// update batcher state (update last uploaded batch block)
218251
*self.last_uploaded_batch_block.lock().await = block_number;
219252

220253
(batch_bytes, batch_merkle_tree.root)

config-files/config-operator-1.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ operator:
2727
metadata_url: "https://yetanotherco.github.io/operator_metadata/metadata.json"
2828
enable_metrics: true
2929
metrics_ip_port_address: localhost:9092
30+
max_batch_size: 268435456 # 256 MiB
31+
3032
# Operators variables needed for register it in EigenLayer
3133
el_delegation_manager_address: "0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9"
3234
private_key_store_path: config-files/devnet/keys/operator-1.ecdsa.key.json

config-files/config-operator-2.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ operator:
2525
delegation_approver_address: "0x0000000000000000000000000000000000000000"
2626
staker_opt_out_window_blocks: 0
2727
metadata_url: "https://yetanotherco.github.io/operator_metadata/metadata.json"
28+
max_batch_size: 268435456 # 256 MiB
29+
2830
# Operators variables needed for register it in EigenLayer
2931
el_delegation_manager_address: "0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9"
3032
private_key_store_path: config-files/devnet/keys/operator-2.ecdsa.key.json

config-files/config-operator-3.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ operator:
2525
delegation_approver_address: "0x0000000000000000000000000000000000000000"
2626
staker_opt_out_window_blocks: 0
2727
metadata_url: "https://yetanotherco.github.io/operator_metadata/metadata.json"
28+
max_batch_size: 268435456 # 256 MiB
29+
2830
# Operators variables needed for register it in EigenLayer
2931
el_delegation_manager_address: "0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9"
3032
private_key_store_path: config-files/devnet/keys/operator-3.ecdsa.key.json

config-files/config.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ bls:
2020
## Batcher configurations
2121
batcher:
2222
block_interval: 3
23-
batch_size_interval: 5
23+
batch_size_interval: 999999999
24+
max_proof_size: 67108864 # 64 MiB
25+
max_batch_size: 268435456 # 256 MiB
2426

2527
## Aggregator Configurations
2628
aggregator:
@@ -40,6 +42,7 @@ operator:
4042
metadata_url: "https://yetanotherco.github.io/operator_metadata/metadata.json"
4143
enable_metrics: true
4244
metrics_ip_port_address: localhost:9092
45+
max_batch_size: 268435456 # 256 MiB
4346
# Operators variables needed for register it in EigenLayer
4447
el_delegation_manager_address: "0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9"
4548
private_key_store_path: config-files/anvil.ecdsa.key.json

core/config/operator.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type OperatorConfig struct {
2424
RegisterOperatorOnStartup bool
2525
EnableMetrics bool
2626
MetricsIpPortAddress string
27+
MaxBatchSize int64
2728
}
2829
}
2930

@@ -38,6 +39,7 @@ type OperatorConfigFromYaml struct {
3839
RegisterOperatorOnStartup bool `yaml:"register_operator_on_startup"`
3940
EnableMetrics bool `yaml:"enable_metrics"`
4041
MetricsIpPortAddress string `yaml:"metrics_ip_port_address"`
42+
MaxBatchSize int64 `yaml:"max_batch_size"`
4143
} `yaml:"operator"`
4244
EcdsaConfigFromYaml EcdsaConfigFromYaml `yaml:"ecdsa"`
4345
BlsConfigFromYaml BlsConfigFromYaml `yaml:"bls"`
@@ -85,6 +87,7 @@ func NewOperatorConfig(configFilePath string) *OperatorConfig {
8587
RegisterOperatorOnStartup bool
8688
EnableMetrics bool
8789
MetricsIpPortAddress string
90+
MaxBatchSize int64
8891
}(operatorConfigFromYaml.Operator),
8992
}
9093
}

operator/pkg/operator.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,8 @@ func (o *Operator) Start(ctx context.Context) error {
133133
// o.Logger.Infof("Received task with index: %d\n", newTaskCreatedLog.TaskIndex)
134134
err := o.ProcessNewBatchLog(newBatchLog)
135135
if err != nil {
136-
o.Logger.Errorf("Proof in batch did not verify", "err", err)
137-
// FIXME(marian): This is not how we should handle this error. Just doing this for fast iteration and debug
138-
panic("Proof did not verify")
136+
o.Logger.Errorf("Batch did not verify", "err", err)
137+
continue
139138
}
140139
responseSignature := o.SignTaskResponse(newBatchLog.BatchMerkleRoot)
141140

@@ -162,7 +161,7 @@ func (o *Operator) ProcessNewBatchLog(newBatchLog *servicemanager.ContractAligne
162161
verificationDataBatch, err := o.getBatchFromS3(newBatchLog.BatchDataPointer)
163162
if err != nil {
164163
o.Logger.Errorf("Could not get proofs from S3 bucket: %v", err)
165-
return nil
164+
return err
166165
}
167166

168167
verificationDataBatchLen := len(verificationDataBatch)

operator/pkg/s3.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,22 @@ import (
99

1010
func (o *Operator) getBatchFromS3(proofUrl string) ([]VerificationData, error) {
1111
o.Logger.Infof("Getting batch from S3..., proofUrl: %s", proofUrl)
12-
resp, err := http.Get(proofUrl)
12+
resp, err := http.Head(proofUrl)
13+
if err != nil {
14+
return nil, err
15+
}
16+
17+
// Check if the response is OK
18+
if resp.StatusCode != http.StatusOK {
19+
return nil, fmt.Errorf("error getting Proof Head from S3: %s", resp.Status)
20+
}
21+
22+
if resp.ContentLength > o.Config.Operator.MaxBatchSize {
23+
return nil, fmt.Errorf("proof size %d exceeds max batch size %d",
24+
resp.ContentLength, o.Config.Operator.MaxBatchSize)
25+
}
26+
27+
resp, err = http.Get(proofUrl)
1328
if err != nil {
1429
return nil, err
1530
}

0 commit comments

Comments
 (0)