-
Notifications
You must be signed in to change notification settings - Fork 57
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
Softfork infrastructure and coinid operator #273
Changes from all commits
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 |
---|---|---|
|
@@ -13,7 +13,7 @@ use crate::err_utils::err; | |
use crate::node::Node; | ||
use crate::number::{number_from_u8, ptr_from_number, Number}; | ||
use crate::op_utils::{ | ||
arg_count, atom, check_arg_count, i32_atom, int_atom, two_ints, u32_from_u8, uint_atom, | ||
arg_count, atom, check_arg_count, i32_atom, int_atom, two_ints, u32_from_u8, | ||
}; | ||
use crate::reduction::{Reduction, Response}; | ||
use crate::sha2::{Digest, Sha256}; | ||
|
@@ -85,6 +85,10 @@ const PUBKEY_BASE_COST: Cost = 1325730; | |
// increased from 12 to closer model Raspberry PI | ||
const PUBKEY_COST_PER_BYTE: Cost = 38; | ||
|
||
// the new coinid operator | ||
const COINID_COST: Cost = | ||
SHA256_BASE_COST + SHA256_COST_PER_ARG * 3 + SHA256_COST_PER_BYTE * (32 + 32 + 8); | ||
|
||
fn limbs_for_int(v: &Number) -> usize { | ||
((v.bits() + 7) / 8) as usize | ||
} | ||
|
@@ -786,23 +790,6 @@ pub fn op_all(a: &mut Allocator, input: NodePtr, max_cost: Cost) -> Response { | |
Ok(Reduction(cost, total.node)) | ||
} | ||
|
||
pub fn op_softfork(a: &mut Allocator, input: NodePtr, max_cost: Cost) -> Response { | ||
let args = Node::new(a, input); | ||
match args.pair() { | ||
Some((p1, _)) => { | ||
let cost = uint_atom::<8>(&p1, "softfork")?; | ||
if cost > max_cost { | ||
return args.err("cost exceeded"); | ||
} | ||
if cost == 0 { | ||
return args.err("cost must be > 0"); | ||
} | ||
Ok(Reduction(cost, args.null().node)) | ||
} | ||
_ => args.err("softfork takes at least 1 argument"), | ||
} | ||
} | ||
|
||
lazy_static! { | ||
static ref GROUP_ORDER: Number = { | ||
let order_as_bytes = &[ | ||
|
@@ -877,3 +864,45 @@ pub fn op_point_add(a: &mut Allocator, input: NodePtr, max_cost: Cost) -> Respon | |
let total: G1Affine = total.into(); | ||
new_atom_and_cost(a, cost, &total.to_compressed()) | ||
} | ||
|
||
pub fn op_coinid(a: &mut Allocator, input: NodePtr, _max_cost: Cost) -> Response { | ||
let args = Node::new(a, input); | ||
check_arg_count(&args, 3, "coinid")?; | ||
|
||
let parent_coin = atom(args.first()?, "coinid")?; | ||
if parent_coin.len() != 32 { | ||
return args.err("coinid: invalid parent coin id (must be 32 bytes)"); | ||
} | ||
let args = args.rest()?; | ||
let puzzle_hash = atom(args.first()?, "coinid")?; | ||
if puzzle_hash.len() != 32 { | ||
return args.err("coinid: invalid puzzle hash (must be 32 bytes)"); | ||
} | ||
let args = args.rest()?; | ||
let amount = atom(args.first()?, "coinid")?; | ||
if !amount.is_empty() { | ||
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 you use 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. yes, I will make this change 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. actually, in this case I want to preserve the buffer, to pass into the Sha256 context, not actually parse the number. I just want to ensure the constraints are met. I could use 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. You might consider checking the assembly generated... it's possible the optimizer might realize you don't use the return value. Also, since it likely just goes into a register anyway, it's probably not particularly expensive. (I may be being a bit naïvely optimistic about how good the optimizers are.) 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 looked into splitting the validation and the parsing of the integer and realized that another important difference is that The only shared check is really that the value is not negative. There are some utility functions to sanitize integers in Even your suggestion of calling |
||
if (amount[0] & 0x80) != 0 { | ||
return args.err("coinid: invalid amount (may not be negative"); | ||
} | ||
if amount == [0_u8] || (amount.len() > 1 && amount[0] == 0 && (amount[1] & 0x80) == 0) { | ||
return args.err("coinid: invalid amount (may not have redundant leading zero)"); | ||
} | ||
// the only valid coin value that's 9 bytes is when a leading zero is | ||
// required to not have the value interpreted as negative | ||
if amount.len() > 9 || (amount.len() == 9 && amount[0] != 0) { | ||
return args.err("coinid: invalid amount (may not exceed max coin amount)"); | ||
} | ||
} | ||
|
||
let mut hasher = Sha256::new(); | ||
hasher.update(parent_coin); | ||
hasher.update(puzzle_hash); | ||
hasher.update(amount); | ||
let ret: [u8; 32] = hasher | ||
.finalize() | ||
.as_slice() | ||
.try_into() | ||
.expect("sha256 hash is not 32 bytes"); | ||
let ret = a.new_atom(&ret)?; | ||
Ok(Reduction(COINID_COST, ret)) | ||
} |
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.
Ah, I see, as we move through the blockchain consensus, we have to adjust how the softfork operator works.
I feel like this can be completely encapsulated in
Dialect
. For simplicity, suppose a soft fork kicks in that introduces a new dialect that supportscoinid
.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.
You would still need some way for the full node to communicate to the rust implementation extensions are available. Before a soft fork activates, the interpreter must behave as if the extension doesn't exist.
Now, in my patch, whether an extension is available or not is controlled by the dialect, see line 137 below. This flag is used to communicate whether the softfork has activated or not.
would you construct a new
ChiaDialect
object for everysoftfork
operator?Presumably you wouldn't start a new interpreter, so you would perhaps have a stack of dialect objects, and execute operators using the top dialect of the stack. The two main issues I see is that it makes the dialect object (and operator dispatch) dynamic rather than static (which is likely to be costly). It's not obvious that having separate classes for the separate dialects really buys us anything, since (presumably) they would be mostly the same.