KIP 17 implementation#797
Conversation
someone235
force-pushed
the
covenants
branch
from
110dcb2 to
0be3dfc
Compare
| } | ||
|
|
||
| pub fn transaction_id_preimage(tx: &Transaction) -> Vec<u8> { | ||
| let mut hasher = PreimageHasher { buff: vec![] }; |
There was a problem hiding this comment.
We can calculate capacity and preallocate vector to avoid allocation when vector grows. However I don't see usages except tests and examples, maybe it's not needed to be done
| impl From<Vec<Vec<u8>>> for Stack { | ||
| fn from(inner: Vec<Vec<u8>>) -> Self { | ||
| // TODO: Replace with the correct max element size after the fork. | ||
| Self { inner, max_element_size: usize::MAX } |
There was a problem hiding this comment.
Isn't it better to set it to 520?
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub(crate) fn get_inner(&self) -> Vec<Vec<u8>> { |
There was a problem hiding this comment.
common name and signature are into_inner(self) or inner(&self) -> &T, in both cases it's up to caller to clone or not
| return Err(TxScriptError::InvalidState("expected boolean".to_string())); | ||
| } | ||
| cond = match cond_buf.pop() { | ||
| Some(stack_cond) => match stack_cond { |
There was a problem hiding this comment.
Can be rewritten with
cond = match cond_buf.pop() {
Some(1) => OpCond::True,
Some(_) => return Err(TxScriptError::InvalidState("expected boolean".to_string())),
None => OpCond::False,
};| return Err(TxScriptError::InvalidState("expected boolean".to_string())); | ||
| } | ||
| cond = match cond_buf.pop() { | ||
| Some(stack_cond) => match stack_cond { |
There was a problem hiding this comment.
Same comment as above regarding "match"
| opcode OpInvert<0x83, 1>(self, vm){ | ||
| if vm.flags.covenants_enabled{ | ||
| let data = vm.dstack.pop()?; | ||
| let r: Vec<u8> = data.iter().map(|b| !b).collect(); |
There was a problem hiding this comment.
It's possible to update data in place data.iter_mut().for_each(|b| *b = !*b);
And push it back onto stack
There was a problem hiding this comment.
I'm using into_iter instead to consume the vector (same for similar comments)
| if a.len() != b.len() { | ||
| return Err(TxScriptError::InvalidState("AND operands must be of equal length".to_string())); | ||
| } | ||
| let r: Vec<u8> = a.iter().zip(b.iter()).map(|(a_byte, b_byte)| a_byte & b_byte).collect(); |
There was a problem hiding this comment.
I think it also makes sense to use iter mut and reuse one of already existing vectors overriding values instead of allocating the new one
| if a.len() != b.len() { | ||
| return Err(TxScriptError::InvalidState("OR operands must be of equal length".to_string())); | ||
| } | ||
| let r: Vec<u8> = a.iter().zip(b.iter()).map(|(a_byte, b_byte)| a_byte | b_byte).collect(); |
| if a.len() != b.len() { | ||
| return Err(TxScriptError::InvalidState("XOR operands must be of equal length".to_string())); | ||
| } | ||
| let r: Vec<u8> = a.iter().zip(b.iter()).map(|(a_byte, b_byte)| a_byte ^ b_byte).collect(); |
| opcode OpMod<0x97, 1>(self, vm){ | ||
| if vm.flags.covenants_enabled{ | ||
| let [ a, b ]: [i64; 2] = vm.dstack.pop_items()?; | ||
| // TODO (before merge): Check with other implementations if they handle negative numbers differently. |
There was a problem hiding this comment.
Is it actual? Anyone use rem_euclid?
There was a problem hiding this comment.
I validated it against BCH code with 1000 i64 numbers drawn randomly, and it seems to be compatible
| let mut next_id: u64 = 1; | ||
| let mut tip = config.genesis.hash; | ||
|
|
||
| // Redeem script that uses OpCat (disabled before covenants activation, enabled after) |
There was a problem hiding this comment.
It doesn't contain opcat
| } | ||
| assert_eq!(consensus.get_virtual_daa_score(), ACTIVATION_DAA_SCORE - 1); | ||
|
|
||
| // Pre-activation: inserting block with the covenant opcode should be rejected |
There was a problem hiding this comment.
Does it use covenant opcode? Actually this block is accepted
2 participants
See KIP 17 for details.
Pending PR for KIP 17: kaspanet/kips#32