Skip to content

Commit 6c352f5

Browse files
fridrik01fridrik01
committed
fix issue with block registry + initial handle success/failure
1 parent e7be2ca commit 6c352f5

File tree

6 files changed

+110
-33
lines changed

6 files changed

+110
-33
lines changed

‎fvm/src/call_manager/default.rs‎

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use anyhow::{anyhow, Context};
66
use cid::Cid;
77
use derive_more::{Deref, DerefMut};
88
use fvm_ipld_amt::Amt;
9-
use fvm_ipld_encoding::{to_vec, CBOR};
9+
use fvm_ipld_encoding::{to_vec, RawBytes, CBOR};
1010
use fvm_shared::address::{Address, Payload};
1111
use fvm_shared::econ::TokenAmount;
1212
use fvm_shared::error::{ErrorNumber, ExitCode};
@@ -263,6 +263,15 @@ where
263263
ExecutionEvent::CallError(SyscallError::new(ErrorNumber::Forbidden, "fatal"))
264264
}
265265
Err(ExecutionError::Syscall(s)) => ExecutionEvent::CallError(s.clone()),
266+
Err(ExecutionError::Abort(Abort::Return(maybe_block))) => {
267+
ExecutionEvent::CallReturn(
268+
ExitCode::OK,
269+
match maybe_block {
270+
Some(block) => RawBytes::new(block.data().to_vec()).into(),
271+
None => RawBytes::default().into(),
272+
},
273+
)
274+
}
266275
Err(ExecutionError::Abort(_)) => {
267276
ExecutionEvent::CallError(SyscallError::new(ErrorNumber::Forbidden, "aborted"))
268277
}
@@ -421,14 +430,8 @@ where
421430
actor_id: ActorID,
422431
new_code_cid: Cid,
423432
params: Option<Block>,
424-
) -> Result<BlockId> {
425-
let result = match self.upgrade_actor_inner::<K>(actor_id, new_code_cid, params) {
426-
Ok(id) => id,
427-
Err(ExecutionError::Abort(Abort::Exit(ExitCode::OK, _, result))) => {
428-
return Ok(result);
429-
}
430-
Err(e) => return Err(e),
431-
};
433+
) -> Result<Option<Block>> {
434+
let result = self.upgrade_actor_inner::<K>(actor_id, new_code_cid, params)?;
432435

433436
let origin = self.origin;
434437
let state = self
@@ -447,21 +450,15 @@ where
447450
),
448451
);
449452

450-
// when we successfully upgrade an actor we want to abort the calling actor which
451-
// made the upgrade and return the block id of the new actor state.
452-
Err(ExecutionError::from(Abort::Exit(
453-
ExitCode::OK,
454-
String::from("actor upgraded"),
455-
result,
456-
)))
453+
Ok(result)
457454
}
458455

459456
fn upgrade_actor_inner<K: Kernel<CallManager = Self>>(
460457
&mut self,
461458
actor_id: ActorID,
462459
new_code_cid: Cid,
463460
params: Option<Block>,
464-
) -> Result<BlockId> {
461+
) -> Result<Option<Block>> {
465462
let state = self
466463
.state_tree()
467464
.get_actor(actor_id)?
@@ -551,7 +548,44 @@ where
551548

552549
let invocation_data = store.into_data();
553550
let _last_error = invocation_data.last_error;
554-
let (cm, _block_registry) = invocation_data.kernel.into_inner();
551+
let (cm, block_registry) = invocation_data.kernel.into_inner();
552+
553+
let result: std::result::Result<Option<Block>, ExecutionError> = match result {
554+
Ok(block_id) => {
555+
if block_id == NO_DATA_BLOCK_ID {
556+
Ok(None)
557+
} else {
558+
Ok(Some(
559+
block_registry
560+
.get(block_id)
561+
.map_err(|_| {
562+
Abort::Exit(
563+
ExitCode::SYS_MISSING_RETURN,
564+
String::from("returned block does not exist"),
565+
NO_DATA_BLOCK_ID,
566+
)
567+
})
568+
.cloned()
569+
.unwrap(),
570+
))
571+
}
572+
}
573+
Err(abort) => match abort {
574+
Abort::Exit(_, _, block_id) => match block_registry.get(block_id) {
575+
Err(e) => Err(ExecutionError::Fatal(anyhow!(
576+
"failed to retrieve block {}: {}",
577+
block_id,
578+
e
579+
))),
580+
Ok(block) => Err(ExecutionError::Abort(Abort::Return(Some(block.clone())))),
581+
},
582+
Abort::OutOfGas => Err(ExecutionError::OutOfGas),
583+
Abort::Fatal(err) => Err(ExecutionError::Fatal(err)),
584+
Abort::Return(_) => todo!(),
585+
},
586+
};
587+
588+
//let ret: std::result::Result<Option<Block>, ExecutionError> = Ok(None);
555589

556590
(result, cm)
557591
})
@@ -967,6 +1001,14 @@ where
9671001
"fatal error".to_owned(),
9681002
Err(ExecutionError::Fatal(err)),
9691003
),
1004+
Abort::Return(value) => (
1005+
ExitCode::OK,
1006+
String::from("aborted"),
1007+
Ok(InvocationResult {
1008+
exit_code: ExitCode::OK,
1009+
value,
1010+
}),
1011+
),
9701012
};
9711013

9721014
if !code.is_success() {

‎fvm/src/call_manager/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ pub trait CallManager: 'static {
116116
actor_id: ActorID,
117117
new_code_cid: Cid,
118118
params: Option<kernel::Block>,
119-
) -> Result<kernel::BlockId>
119+
) -> Result<Option<kernel::Block>>
120120
where
121121
K: Kernel<CallManager = Self>;
122122

@@ -125,7 +125,7 @@ pub trait CallManager: 'static {
125125
actor_id: ActorID,
126126
new_code_cid: Cid,
127127
params: Option<kernel::Block>,
128-
) -> Result<kernel::BlockId>
128+
) -> Result<Option<kernel::Block>>
129129
where
130130
K: Kernel<CallManager = Self>;
131131

‎fvm/src/kernel/blocks.rs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright 2021-2023 Protocol Labs
22
// SPDX-License-Identifier: Apache-2.0, MIT
33
use std::convert::TryInto;
4-
use std::rc::Rc;
54

65
use fvm_ipld_encoding::ipld_block::IpldBlock;
76
use fvm_ipld_encoding::{CBOR, DAG_CBOR};
@@ -36,10 +35,7 @@ pub struct BlockStat {
3635
#[derive(Debug, Clone)]
3736
pub struct Block {
3837
codec: u64,
39-
// Unfortunately, we usually start with a vector/boxed buffer. If we used Rc<[u8]>, we'd have to
40-
// copy the bytes. So we accept some indirection for reliable performance.
41-
#[allow(clippy::redundant_allocation)]
42-
data: Rc<Box<[u8]>>,
38+
data: Box<[u8]>,
4339
}
4440

4541
impl Block {
@@ -48,7 +44,7 @@ impl Block {
4844
// The extra allocation is basically nothing.
4945
Self {
5046
codec,
51-
data: Rc::new(data.into()),
47+
data: data.into(),
5248
}
5349
}
5450

@@ -92,7 +88,7 @@ impl From<&Block> for IpldBlock {
9288
fn from(b: &Block) -> Self {
9389
IpldBlock {
9490
codec: b.codec,
95-
data: Vec::from(&**b.data),
91+
data: Vec::from(&*b.data),
9692
}
9793
}
9894
}

‎fvm/src/kernel/default.rs‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::convert::{TryFrom, TryInto};
55
use std::panic::{self, UnwindSafe};
66
use std::path::PathBuf;
77

8+
use crate::syscalls::error::Abort;
89
use anyhow::{anyhow, Context as _};
910
use blake2b_simd::Params;
1011
use byteorder::WriteBytesExt;
@@ -18,7 +19,7 @@ use fvm_shared::chainid::ChainID;
1819
use fvm_shared::consensus::ConsensusFault;
1920
use fvm_shared::crypto::signature;
2021
use fvm_shared::econ::TokenAmount;
21-
use fvm_shared::error::ErrorNumber;
22+
use fvm_shared::error::{ErrorNumber, ExitCode};
2223
use fvm_shared::event::ActorEvent;
2324
use fvm_shared::piece::{zero_piece_commitment, PaddedPieceSize};
2425
use fvm_shared::sector::RegisteredPoStProof::{StackedDRGWindow32GiBV1, StackedDRGWindow32GiBV1P1};
@@ -887,8 +888,37 @@ where
887888
Some(self.blocks.get(params_id)?.clone())
888889
};
889890

890-
self.call_manager
891-
.upgrade_actor::<Self>(self.actor_id, new_code_cid, params)
891+
let result = self
892+
.call_manager
893+
.upgrade_actor::<Self>(self.actor_id, new_code_cid, params);
894+
895+
if let Ok(None) = result {
896+
Err(ExecutionError::Abort(Abort::Exit(
897+
ExitCode::OK,
898+
"actor upgraded".to_string(),
899+
NO_DATA_BLOCK_ID,
900+
)))
901+
} else if let Ok(Some(block)) = result {
902+
let block_id = self
903+
.blocks
904+
.put(block)
905+
.or_fatal()
906+
.context("failed to store a valid return value")?;
907+
Err(ExecutionError::Abort(Abort::Exit(
908+
ExitCode::OK,
909+
"actor upgraded".to_string(),
910+
block_id,
911+
)))
912+
} else if let Err(ExecutionError::Abort(Abort::Return(Some(block)))) = result {
913+
let block_id = self
914+
.blocks
915+
.put(block)
916+
.or_fatal()
917+
.context("failed to store a valid return value")?;
918+
Ok(block_id)
919+
} else {
920+
Err(result.unwrap_err())
921+
}
892922
}
893923

894924
fn get_builtin_actor_type(&self, code_cid: &Cid) -> Result<u32> {

‎fvm/src/syscalls/error.rs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use fvm_shared::error::ExitCode;
66
use wasmtime::Trap;
77

88
use crate::call_manager::NO_DATA_BLOCK_ID;
9-
use crate::kernel::{BlockId, ExecutionError};
9+
use crate::kernel::{Block, BlockId, ExecutionError};
1010

1111
/// Represents an actor "abort".
1212
#[derive(Debug, thiserror::Error)]
@@ -20,6 +20,9 @@ pub enum Abort {
2020
/// The system failed with a fatal error.
2121
#[error("fatal error: {0}")]
2222
Fatal(anyhow::Error),
23+
/// The actor aborted with a block.
24+
#[error("abortive non-local return {0:?}")]
25+
Return(Option<Block>),
2326
}
2427

2528
impl Abort {
@@ -80,9 +83,15 @@ impl From<anyhow::Error> for Abort {
8083
_ => Abort::Fatal(anyhow!("unexpected wasmtime trap: {}", trap)),
8184
};
8285
};
86+
8387
match e.downcast::<Abort>() {
8488
Ok(abort) => abort,
8589
Err(e) => Abort::Fatal(e),
8690
}
91+
92+
/*match e.source().and_then(|e| e.downcast_mut::<Abort>()) {
93+
Some(abort) => abort,
94+
None => Abort::Fatal(e),
95+
}*/
8796
}
8897
}

‎fvm/tests/dummy.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ impl CallManager for DummyCallManager {
398398
_actor_id: ActorID,
399399
_new_code_cid: Cid,
400400
_params: Option<kernel::Block>,
401-
) -> kernel::Result<kernel::BlockId>
401+
) -> kernel::Result<Option<kernel::Block>>
402402
where
403403
K: Kernel<CallManager = Self>,
404404
{
@@ -410,7 +410,7 @@ impl CallManager for DummyCallManager {
410410
_actor_id: ActorID,
411411
_new_code_cid: Cid,
412412
_params: Option<kernel::Block>,
413-
) -> kernel::Result<kernel::BlockId>
413+
) -> kernel::Result<Option<kernel::Block>>
414414
where
415415
K: Kernel<CallManager = Self>,
416416
{

0 commit comments

Comments
 (0)