Skip to content

Commit b158415

Browse files
chenyukangeval-exec
chenyukang
and
eval-exec
authored
Merge commit from fork
* fix malformed block uncle response panic * remove expect in reconstruct_block * validate deposit header block number in DAO withdraw calculation * style: format DAO withdraw validation changes * fix since timestamp panic * handle overflow risk * fix overflow issue in since * more overflow check for block_assembler * more overflow handling on scheduler * test: cover syscall address overflow cases * style: apply rustfmt * fix performance of transaction_maximum_withdraw --------- Co-authored-by: Eval Exec <execvy@gmail.com>
1 parent 8ed59ca commit b158415

35 files changed

Lines changed: 988 additions & 199 deletions

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎chain/src/init_load_unverified.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::{ChainController, LonelyBlock};
33
use ckb_constant::sync::BLOCK_DOWNLOAD_WINDOW;
44
use ckb_db::{Direction, IteratorMode};
55
use ckb_db_schema::COLUMN_NUMBER_HASH;
6-
use ckb_logger::info;
6+
use ckb_logger::{error, info};
77
use ckb_shared::Shared;
88
use ckb_stop_handler::has_received_stop_signal;
99
use ckb_store::ChainStore;
@@ -81,7 +81,14 @@ impl InitLoadUnverified {
8181
1,
8282
tip_number.saturating_sub(EXPIRED_EPOCH * self.shared.consensus().max_epoch_length()),
8383
);
84-
let end_check_number = tip_number + BLOCK_DOWNLOAD_WINDOW * 10;
84+
let Some(end_check_number) = tip_number.checked_add(BLOCK_DOWNLOAD_WINDOW * 10) else {
85+
error!(
86+
"unverified block scan end overflows: tip_number {}, window {}",
87+
tip_number,
88+
BLOCK_DOWNLOAD_WINDOW * 10
89+
);
90+
return;
91+
};
8592

8693
for check_unverified_number in start_check_number..=end_check_number {
8794
if has_received_stop_signal() {

‎chain/src/verify.rs‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ use ckb_verification::cache::Completed;
2424
use ckb_verification_contextual::{ContextualBlockVerifier, VerifyContext};
2525
use ckb_verification_traits::Switch;
2626
use dashmap::DashSet;
27+
use std::any::Any;
2728
use std::cmp;
2829
use std::collections::HashSet;
30+
use std::panic::{AssertUnwindSafe, catch_unwind};
2931
use std::sync::Arc;
3032

3133
pub(crate) struct ConsumeUnverifiedBlockProcessor {
@@ -79,7 +81,19 @@ impl ConsumeUnverifiedBlocks {
7981
let _ = self.tx_pool_controller.suspend_chunk_process();
8082

8183
let _trace_now = minstant::Instant::now();
82-
self.processor.consume_unverified_blocks(unverified_task);
84+
let block_hash = unverified_task.block.hash();
85+
let block_number = unverified_task.block.number();
86+
if let Err(payload) = catch_unwind(AssertUnwindSafe(|| {
87+
self.processor.consume_unverified_blocks(unverified_task);
88+
})) {
89+
error!(
90+
"consume unverified block {}-{} panicked: {}",
91+
block_number,
92+
block_hash,
93+
panic_payload_to_string(payload.as_ref())
94+
);
95+
self.processor.is_pending_verify.remove(&block_hash);
96+
}
8397
if let Some(handle) = ckb_metrics::handle() {
8498
handle.ckb_chain_consume_unverified_block_duration.observe(_trace_now.elapsed().as_secs_f64())
8599
}
@@ -893,6 +907,16 @@ impl ConsumeUnverifiedBlockProcessor {
893907
}
894908
}
895909

910+
fn panic_payload_to_string(payload: &(dyn Any + Send)) -> String {
911+
if let Some(message) = payload.downcast_ref::<&str>() {
912+
(*message).to_owned()
913+
} else if let Some(message) = payload.downcast_ref::<String>() {
914+
message.clone()
915+
} else {
916+
"non-string panic payload".to_owned()
917+
}
918+
}
919+
896920
#[cfg(debug_assertions)]
897921
fn is_sorted_assert(fork: &ForkChanges) {
898922
assert!(fork.is_sorted())

‎rpc/src/tests/fee_rate.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,30 @@ fn test_fee_rate_statics() {
114114
})
115115
);
116116
}
117+
118+
#[test]
119+
fn test_fee_rate_statics_handles_large_values() {
120+
let mut provider = DummyFeeRateProvider::new(3);
121+
for i in 1..=2 {
122+
provider.append(
123+
i,
124+
BlockExt {
125+
received_at: 0,
126+
total_difficulty: 0u64.into(),
127+
total_uncles_count: 0,
128+
verified: None,
129+
txs_fees: vec![Capacity::shannons(u64::MAX)],
130+
cycles: Some(vec![0]),
131+
txs_sizes: Some(vec![0, 1]),
132+
},
133+
);
134+
}
135+
136+
assert_eq!(
137+
FeeRateCollector::new(&provider).statistics(Some(3)),
138+
Some(FeeRateStatistics {
139+
mean: u64::MAX.into(),
140+
median: u64::MAX.into(),
141+
})
142+
);
143+
}

‎rpc/src/util/fee_rate.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ fn is_even(n: u64) -> bool {
1212
}
1313

1414
fn mean(numbers: &[u64]) -> u64 {
15-
let sum: u64 = numbers.iter().sum();
16-
sum / numbers.len() as u64
15+
// The average of u64 values fits in u64, but the intermediate sum may not.
16+
let sum: u128 = numbers.iter().map(|number| u128::from(*number)).sum();
17+
(sum / numbers.len() as u128) as u64
1718
}
1819

1920
fn median(numbers: &mut [u64]) -> u64 {

‎script/src/scheduler.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,10 @@ where
690690
let copy_length = u64::min(full_length, real_length);
691691
for i in 0..copy_length {
692692
let fd = inherited_fd[i as usize].0;
693-
let addr = buffer_addr.checked_add(i * 8).ok_or(Error::MemOutOfBound)?;
693+
let offset = i.checked_mul(8).ok_or(Error::MemOutOfBound)?;
694+
let addr = buffer_addr
695+
.checked_add(offset)
696+
.ok_or(Error::MemOutOfBound)?;
694697
machine
695698
.inner_mut()
696699
.memory_mut()
@@ -810,10 +813,12 @@ where
810813
write_machine
811814
.inner_mut()
812815
.add_cycles_no_checking(transferred_byte_cycles(copiable))?;
813-
let data = write_machine
814-
.inner_mut()
815-
.memory_mut()
816-
.load_bytes(write_buffer_addr.wrapping_add(consumed), copiable)?;
816+
let data = write_machine.inner_mut().memory_mut().load_bytes(
817+
write_buffer_addr
818+
.checked_add(consumed)
819+
.ok_or(Error::MemOutOfBound)?,
820+
copiable,
821+
)?;
817822
let (_, read_machine) = self
818823
.instantiated
819824
.get_mut(&read_vm_id)

‎script/src/syscalls/debugger.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
use crate::types::{
22
DebugPrinter, {SgData, SgInfo},
33
};
4-
use crate::{cost_model::transferred_byte_cycles, syscalls::DEBUG_PRINT_SYSCALL_NUMBER};
4+
use crate::{
5+
cost_model::transferred_byte_cycles,
6+
syscalls::{DEBUG_PRINT_SYSCALL_NUMBER, utils::checked_add_addr},
7+
};
58
use ckb_vm::{
69
Error as VMError, Memory, Register, SupportMachine, Syscalls,
710
registers::{A0, A7},
@@ -45,7 +48,7 @@ impl<Mac: SupportMachine> Syscalls<Mac> for Debugger {
4548
break;
4649
}
4750
buffer.push(byte);
48-
addr += 1;
51+
addr = checked_add_addr(addr, 1)?;
4952
}
5053

5154
machine.add_cycles_no_checking(transferred_byte_cycles(buffer.len() as u64))?;

‎script/src/syscalls/exec.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::cost_model::transferred_byte_cycles;
22
use crate::syscalls::{
33
EXEC, INDEX_OUT_OF_BOUND, MAX_ARGV_LENGTH, Place, SLICE_OUT_OF_BOUND, Source, SourceEntry,
4-
WRONG_FORMAT,
4+
WRONG_FORMAT, utils::checked_add_addr,
55
};
66
use crate::types::SgData;
77
use ckb_traits::CellDataProvider;
@@ -169,7 +169,7 @@ impl<Mac: SupportMachine, DL: CellDataProvider + Send + Sync + Clone> Syscalls<M
169169
return Err(VMError::Unexpected(ARGV_TOO_LONG_TEXT.to_string()));
170170
}
171171

172-
addr += 8;
172+
addr = checked_add_addr(addr, 8)?;
173173
}
174174

175175
let cycles = machine.cycles();

‎script/src/syscalls/pipe.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::syscalls::{PIPE, SPAWN_YIELD_CYCLES_BASE};
1+
use crate::syscalls::{PIPE, SPAWN_YIELD_CYCLES_BASE, utils::checked_add_addr};
22
use crate::types::{Message, PipeArgs, VmContext, VmId};
33
use ckb_traits::{CellDataProvider, ExtensionProvider, HeaderProvider};
44
use ckb_vm::{
@@ -35,7 +35,7 @@ impl<Mac: SupportMachine> Syscalls<Mac> for Pipe {
3535
return Ok(false);
3636
}
3737
let fd1_addr = machine.registers()[A0].to_u64();
38-
let fd2_addr = fd1_addr.wrapping_add(8);
38+
let fd2_addr = checked_add_addr(fd1_addr, 8)?;
3939
machine.add_cycles_no_checking(SPAWN_YIELD_CYCLES_BASE)?;
4040
self.message_box
4141
.lock()

‎script/src/syscalls/spawn.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::syscalls::{
22
INDEX_OUT_OF_BOUND, SLICE_OUT_OF_BOUND, SOURCE_ENTRY_MASK, SOURCE_GROUP_FLAG, SPAWN,
3-
SPAWN_EXTRA_CYCLES_BASE, SPAWN_YIELD_CYCLES_BASE, Source,
3+
SPAWN_EXTRA_CYCLES_BASE, SPAWN_YIELD_CYCLES_BASE, Source, utils::checked_add_addr,
44
};
55
use crate::types::{DataLocation, DataPieceId, Fd, Message, SgData, SpawnArgs, VmContext, VmId};
66
use ckb_traits::{CellDataProvider, ExtensionProvider, HeaderProvider};
@@ -74,16 +74,16 @@ where
7474
let argc = machine
7575
.memory_mut()
7676
.load64(&Mac::REG::from_u64(argc_addr))?;
77-
let argv_addr = spgs_addr.wrapping_add(8);
77+
let argv_addr = checked_add_addr(spgs_addr, 8)?;
7878
let argv = machine
7979
.memory_mut()
8080
.load64(&Mac::REG::from_u64(argv_addr))?;
81-
let process_id_addr_addr = spgs_addr.wrapping_add(16);
81+
let process_id_addr_addr = checked_add_addr(spgs_addr, 16)?;
8282
let process_id_addr = machine
8383
.memory_mut()
8484
.load64(&Mac::REG::from_u64(process_id_addr_addr))?
8585
.to_u64();
86-
let fds_addr_addr = spgs_addr.wrapping_add(24);
86+
let fds_addr_addr = checked_add_addr(spgs_addr, 24)?;
8787
let mut fds_addr = machine
8888
.memory_mut()
8989
.load64(&Mac::REG::from_u64(fds_addr_addr))?
@@ -100,7 +100,7 @@ where
100100
break;
101101
}
102102
fds.push(Fd(fd));
103-
fds_addr += 8;
103+
fds_addr = checked_add_addr(fds_addr, 8)?;
104104
}
105105
}
106106

0 commit comments

Comments
 (0)