Skip to content
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

Remove chia-blockchain dependency for most of chia_rs #887

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
fe7819a
remove imports from test_blscache and delete test_program_fidelity
matt-o-how Jan 17, 2025
315fab0
remove imports from block_record by implement pot_iterations
matt-o-how Jan 17, 2025
0d406e7
clippy and fmt
matt-o-how Jan 17, 2025
d27c1a5
don't export new functions
matt-o-how Jan 17, 2025
19ca423
black tests
matt-o-how Jan 17, 2025
264058d
import PyValueError at top of file
matt-o-how Jan 17, 2025
719b8a2
add pot_iterations and pos_quality modules; refactor block_record to …
matt-o-how Jan 22, 2025
74a700e
fix circular import
matt-o-how Jan 22, 2025
ef8c936
temp commit
matt-o-how Jan 22, 2025
06cdaad
fix build with py-bindings
matt-o-how Jan 22, 2025
2b9d0f7
clippy and fmt
matt-o-how Jan 22, 2025
242c993
black format test
matt-o-how Jan 22, 2025
b8c0305
fix fetching rust values from python
matt-o-how Jan 23, 2025
7d38538
tests run but overflow
matt-o-how Jan 24, 2025
2bb5c1e
correct order of blockrecord args
matt-o-how Jan 24, 2025
31bdbc7
add tests to pot_iterations.rs
matt-o-how Jan 24, 2025
a1e9425
fix bug in implementation
matt-o-how Jan 27, 2025
6251c19
fix python test
matt-o-how Jan 27, 2025
6645f85
fmt and black
matt-o-how Jan 27, 2025
fb5897e
add ip_iters pytest
matt-o-how Jan 27, 2025
44a04d3
clippy changes
matt-o-how Jan 27, 2025
d752a28
update type stubs for new python functions
matt-o-how Jan 27, 2025
d0d0883
fix stubs
matt-o-how Jan 27, 2025
b39c89c
add expected_plot_size to api.rs
matt-o-how Jan 27, 2025
638943b
black test
matt-o-how Jan 27, 2025
4294ff4
process signage_point_index as u32 to prevent overflows
matt-o-how Jan 27, 2025
68aca23
pytests passing
matt-o-how Jan 27, 2025
9f6f641
black test again
matt-o-how Jan 27, 2025
5c1c692
use sized ints in stub definition
matt-o-how Feb 7, 2025
966daa4
Remove commented-out test
matt-o-how Feb 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 82 additions & 59 deletions crates/chia-protocol/src/block_record.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a really nice change. However, I think it would be good to check the correctness of this against the existing python implementation. One way to do that would be to break this off into a separate PR that still relies on the chia-blockchain dependency to compare the result between the rust and python implementation. Can you think of another way to be confident in the correctness of this port?

Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use chia_streamable_macro::streamable;

#[cfg(feature = "py-bindings")]
use crate::pot_iterations::{calculate_ip_iters, calculate_sp_iters};
use crate::{Bytes32, ClassgroupElement, Coin, SubEpochSummary};
use chia_streamable_macro::streamable;
#[cfg(feature = "py-bindings")]
use pyo3::exceptions::PyValueError;

#[cfg(feature = "py-bindings")]
use pyo3::prelude::*;
Expand Down Expand Up @@ -67,13 +70,48 @@ impl BlockRecord {
pub fn is_challenge_block(&self, min_blocks_per_challenge_block: u8) -> bool {
self.deficit == min_blocks_per_challenge_block - 1
}
}

#[cfg(feature = "py-bindings")]
use pyo3::types::PyDict;

#[cfg(feature = "py-bindings")]
use pyo3::exceptions::PyValueError;
// fn calculate_sp_interval_iters(&self, num_sps_sub_slot: u64) -> PyResult<u64> {
// if self.sub_slot_iters % num_sps_sub_slot != 0 {
// return Err(PyValueError::new_err(
// "sub_slot_iters % constants.NUM_SPS_SUB_SLOT != 0",
// ));
// }
// Ok(self.sub_slot_iters / num_sps_sub_slot)
// }

// fn calculate_sp_iters(&self, num_sps_sub_slot: u32) -> PyResult<u64> {
// if self.signage_point_index as u32 >= num_sps_sub_slot {
// return Err(PyValueError::new_err("SP index too high"));
// }
// Ok(self.calculate_sp_interval_iters(num_sps_sub_slot as u64)?
// * self.signage_point_index as u64)
// }

// fn calculate_ip_iters(
// &self,
// num_sps_sub_slot: u32,
// num_sp_intervals_extra: u8,
// ) -> PyResult<u64> {
// let sp_iters = self.calculate_sp_iters(num_sps_sub_slot)?;
// let sp_interval_iters = self.calculate_sp_interval_iters(num_sps_sub_slot as u64)?;
// if sp_iters % sp_interval_iters != 0 || sp_iters >= self.sub_slot_iters {
// return Err(PyValueError::new_err(format!(
// "Invalid sp iters {sp_iters} for this ssi {}",
// self.sub_slot_iters
// )));
// } else if self.required_iters >= sp_interval_iters || self.required_iters == 0 {
// return Err(PyValueError::new_err(format!(
// "Required iters {} is not below the sp interval iters {} {} or not >=0",
// self.required_iters, sp_interval_iters, self.sub_slot_iters
// )));
// }
// Ok(
// (sp_iters + num_sp_intervals_extra as u64 * sp_interval_iters + self.required_iters)
// % self.sub_slot_iters,
// )
// }
}

#[cfg(feature = "py-bindings")]
use chia_traits::ChiaToPython;
Expand Down Expand Up @@ -102,16 +140,11 @@ impl BlockRecord {
))
}

// TODO: at some point it would be nice to port
// chia.consensus.pot_iterations to rust, and make this less hacky
fn sp_sub_slot_total_iters_impl(
&self,
py: Python<'_>,
constants: &Bound<'_, PyAny>,
) -> PyResult<u128> {
// TODO: these could be implemented as a total port of pot iterations
fn sp_sub_slot_total_iters_impl(&self, constants: &Bound<'_, PyAny>) -> PyResult<u128> {
let ret = self
.total_iters
.checked_sub(self.ip_iters_impl(py, constants)? as u128)
.checked_sub(self.ip_iters_impl(constants)? as u128)
.ok_or(PyValueError::new_err("uint128 overflow"))?;
if self.overflow {
ret.checked_sub(self.sub_slot_iters as u128)
Expand All @@ -121,48 +154,38 @@ impl BlockRecord {
}
}

fn ip_sub_slot_total_iters_impl(
&self,
py: Python<'_>,
constants: &Bound<'_, PyAny>,
) -> PyResult<u128> {
fn ip_sub_slot_total_iters_impl(&self, constants: &Bound<'_, PyAny>) -> PyResult<u128> {
self.total_iters
.checked_sub(self.ip_iters_impl(py, constants)? as u128)
.checked_sub(self.ip_iters_impl(constants)? as u128)
.ok_or(PyValueError::new_err("uint128 overflow"))
}

fn sp_iters_impl(&self, py: Python<'_>, constants: &Bound<'_, PyAny>) -> PyResult<u64> {
let ctx = PyDict::new(py);
ctx.set_item("sub_slot_iters", self.sub_slot_iters)?;
ctx.set_item("signage_point_index", self.signage_point_index)?;
ctx.set_item("constants", constants)?;
py.run(
c"from chia.consensus.pot_iterations import calculate_ip_iters, calculate_sp_iters\n\
ret = calculate_sp_iters(constants, sub_slot_iters, signage_point_index)\n",
None,
Some(&ctx),
)?;
ctx.get_item("ret").unwrap().unwrap().extract::<u64>()
}

fn ip_iters_impl(&self, py: Python<'_>, constants: &Bound<'_, PyAny>) -> PyResult<u64> {
let ctx = PyDict::new(py);
ctx.set_item("sub_slot_iters", self.sub_slot_iters)?;
ctx.set_item("signage_point_index", self.signage_point_index)?;
ctx.set_item("required_iters", self.required_iters)?;
ctx.set_item("constants", constants)?;
py.run(
c"from chia.consensus.pot_iterations import calculate_ip_iters, calculate_sp_iters\n\
ret = calculate_ip_iters(constants, sub_slot_iters, signage_point_index, required_iters)\n",
None,
Some(&ctx),
)?;
ctx.get_item("ret").unwrap().unwrap().extract::<u64>()
}

fn sp_total_iters_impl(&self, py: Python<'_>, constants: &Bound<'_, PyAny>) -> PyResult<u128> {
self.sp_sub_slot_total_iters_impl(py, constants)?
.checked_add(self.sp_iters_impl(py, constants)? as u128)
fn sp_iters_impl(&self, constants: &Bound<'_, PyAny>) -> PyResult<u64> {
let num_sps_sub_slot = constants.getattr("NUM_SPS_SUB_SLOT")?.extract::<u32>()?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that these are implemented in rust, I don't think they should be limited to just the python bindings anymore. I imagine you put them here to simplify error handling, so make them return PyResult<>. If you make them part of the rust type and make them return chia_error::Result<> you could map those errors with map_err()

calculate_sp_iters(
num_sps_sub_slot,
self.sub_slot_iters,
self.signage_point_index as u32,
)
}

fn ip_iters_impl(&self, constants: &Bound<'_, PyAny>) -> PyResult<u64> {
let num_sps_sub_slot = constants.getattr("NUM_SPS_SUB_SLOT")?.extract::<u32>()?;
let num_sp_intervals_extra = constants
.getattr("NUM_SP_INTERVALS_EXTRA")?
.extract::<u8>()?;
calculate_ip_iters(
num_sps_sub_slot,
num_sp_intervals_extra,
self.sub_slot_iters,
self.signage_point_index as u32,
self.required_iters,
)
}

fn sp_total_iters_impl(&self, constants: &Bound<'_, PyAny>) -> PyResult<u128> {
self.sp_sub_slot_total_iters_impl(constants)?
.checked_add(self.sp_iters_impl(constants)? as u128)
.ok_or(PyValueError::new_err("uint128 overflow"))
}

Expand All @@ -171,38 +194,38 @@ impl BlockRecord {
py: Python<'a>,
constants: &Bound<'_, PyAny>,
) -> PyResult<Bound<'a, PyAny>> {
ChiaToPython::to_python(&self.sp_sub_slot_total_iters_impl(py, constants)?, py)
ChiaToPython::to_python(&self.sp_sub_slot_total_iters_impl(constants)?, py)
}

fn ip_sub_slot_total_iters<'a>(
&self,
py: Python<'a>,
constants: &Bound<'_, PyAny>,
) -> PyResult<Bound<'a, PyAny>> {
ChiaToPython::to_python(&self.ip_sub_slot_total_iters_impl(py, constants)?, py)
ChiaToPython::to_python(&self.ip_sub_slot_total_iters_impl(constants)?, py)
}

fn sp_iters<'a>(
&self,
py: Python<'a>,
constants: &Bound<'_, PyAny>,
) -> PyResult<Bound<'a, PyAny>> {
ChiaToPython::to_python(&self.sp_iters_impl(py, constants)?, py)
ChiaToPython::to_python(&self.sp_iters_impl(constants)?, py)
}

fn ip_iters<'a>(
&self,
py: Python<'a>,
constants: &Bound<'_, PyAny>,
) -> PyResult<Bound<'a, PyAny>> {
ChiaToPython::to_python(&self.ip_iters_impl(py, constants)?, py)
ChiaToPython::to_python(&self.ip_iters_impl(constants)?, py)
}

fn sp_total_iters<'a>(
&self,
py: Python<'a>,
constants: &Bound<'_, PyAny>,
) -> PyResult<Bound<'a, PyAny>> {
ChiaToPython::to_python(&self.sp_total_iters_impl(py, constants)?, py)
ChiaToPython::to_python(&self.sp_total_iters_impl(constants)?, py)
}
}
8 changes: 8 additions & 0 deletions crates/chia-protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ mod fullblock;
mod header_block;
mod peer_info;
mod pool_target;
#[cfg(feature = "py-bindings")]
mod pos_quality;
#[cfg(feature = "py-bindings")]
mod pot_iterations;
mod program;
mod proof_of_space;
mod reward_chain_block;
Expand Down Expand Up @@ -44,6 +48,10 @@ pub use crate::fullblock::*;
pub use crate::header_block::*;
pub use crate::peer_info::*;
pub use crate::pool_target::*;
#[cfg(feature = "py-bindings")]
pub use crate::pos_quality::*;
#[cfg(feature = "py-bindings")]
pub use crate::pot_iterations::*;
pub use crate::program::*;
pub use crate::proof_of_space::*;
pub use crate::reward_chain_block::*;
Expand Down
19 changes: 19 additions & 0 deletions crates/chia-protocol/src/pos_quality.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// The actual space in bytes of a plot, is _expected_plot_size(k) * UI_ACTUAL_SPACE_CONSTANT_FACTO
// This is not used in consensus, only for display purposes

pub const UI_ACTUAL_SPACE_CONSTANT_FACTOR: f32 = 0.78;

// TODO: Update this when new plot format releases
#[cfg(feature = "py-bindings")]
#[pyo3::pyfunction]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have to do #[cfg_attr(feature = "py-bindings", pyo3::pyfunction)] instead?

pub fn expected_plot_size(k: u32) -> pyo3::PyResult<u64> {
// """
// Given the plot size parameter k (which is between 32 and 59), computes the
// expected size of the plot in bytes (times a constant factor). This is based on efficient encoding
// of the plot, and aims to be scale agnostic, so larger plots don't
// necessarily get more rewards per byte. The +1 is added to give half a bit more space per entry, which
// is necessary to store the entries in the plot.
// """

Ok((2 * k as u64 + 1) * (1_u64 << (k - 1)))
}
Loading
Loading