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

feat: add new get_script_hash kernel procedure #995

Merged
merged 11 commits into from
Dec 2, 2024
9 changes: 9 additions & 0 deletions miden-lib/asm/kernels/transaction/api.masm
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,15 @@ export.get_note_serial_number
# => [SERIAL_NUMBER, pad(12)]
end

export.get_note_script_hash
exec.note::get_note_script_hash
# => [SCRIPT_HASH, pad(16)]

# truncate the stack
swapw dropw
# => [SCRIPT_HASH, pad(12)]
end

#! Tells the transaction kernel that we are about to execute a procedure on a foreign account.
#!
#! Checks whether the current foreign account was already loaded to the memory, and loads it if not.
Expand Down
12 changes: 12 additions & 0 deletions miden-lib/asm/kernels/transaction/lib/note.masm
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,18 @@ export.prepare_note
# => [note_script_root_ptr, NOTE_ARGS]
end

#! Returns the current note script hash.
#!
#! Inputs: []
#! Outputs: [SCRIPT_HASH]
export.get_note_script_hash
exec.memory::get_current_input_note_ptr
# => [note_ptr]

exec.memory::get_input_note_script_root
# => [SCRIPT_HASH]
end

# OUTPUT NOTE PROCEDURES
# =================================================================================================

Expand Down
19 changes: 12 additions & 7 deletions miden-lib/asm/miden/kernel_proc_offsets.masm
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ const.GET_NOTE_ASSETS_INFO_OFFSET=21
const.GET_NOTE_INPUTS_HASH_OFFSET=22
const.GET_NOTE_SENDER_OFFSET=23
const.GET_NOTE_SERIAL_NUMBER_OFFSET=24
const.GET_OUTPUT_NOTES_COMMITMENT_OFFSET=25
const.GET_NOTE_SCRIPT_HASH_OFFSET=25
const.GET_OUTPUT_NOTES_COMMITMENT_OFFSET=26

# Transaction
const.GET_BLOCK_HASH_OFFSET=26
const.GET_BLOCK_NUMBER_OFFSET=27
const.START_FOREIGN_CONTEXT_OFFSET=28
const.END_FOREIGN_CONTEXT_OFFSET=29
const.UPDATE_EXPIRATION_BLOCK_NUM_OFFSET=30
const.GET_EXPIRATION_DELTA_OFFSET=31
const.GET_BLOCK_HASH_OFFSET=27
const.GET_BLOCK_NUMBER_OFFSET=28
const.START_FOREIGN_CONTEXT_OFFSET=29
const.END_FOREIGN_CONTEXT_OFFSET=30
const.UPDATE_EXPIRATION_BLOCK_NUM_OFFSET=31
const.GET_EXPIRATION_DELTA_OFFSET=32

# ACCESSORS
# -------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -404,6 +405,10 @@ export.get_note_serial_number_offset
push.GET_NOTE_SERIAL_NUMBER_OFFSET
end

export.get_note_script_hash_offset
push.GET_NOTE_SCRIPT_HASH_OFFSET
end

#! Returns an offset of the `start_foreign_context` kernel procedure.
#!
#! Inputs: []
Expand Down
16 changes: 16 additions & 0 deletions miden-lib/asm/miden/note.masm
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,22 @@ export.compute_inputs_hash
# => [HASH]
end

export.get_note_script_hash
# pad the stack
padw padw padw push.0.0.0
# => [pad(15)]

exec.kernel_proc_offsets::get_note_script_hash_offset
# => [offset, pad(15)]

syscall.exec_kernel_proc
# => [SCRIPT_HASH, pad(12)]

# clean the stack
swapdw dropw dropw swapw dropw
# => [SCRIPT_HASH]
end

# PROCEDURES COPIED FROM KERNEL (TODO: get rid of this duplication)
# =================================================================================================

Expand Down
31 changes: 30 additions & 1 deletion miden-tx/src/tests/kernel_tests/test_note.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use miden_lib::{
use miden_objects::{
notes::Note, testing::prepare_word, transaction::TransactionArgs, Hasher, WORD_SIZE,
};
use vm_processor::{ProcessState, EMPTY_WORD, ONE};
use vm_processor::{ProcessState, Word, EMPTY_WORD, ONE};

use super::{Felt, Process, ZERO};
use crate::{
Expand Down Expand Up @@ -520,3 +520,32 @@ fn test_get_inputs_hash() {

assert_eq!(process.get_stack_state()[0..16], expected_stack);
}

#[test]
fn test_get_current_script_hash() {
let tx_context = TransactionContextBuilder::with_standard_account(ONE)
.with_mock_notes_preserved()
.build();

// calling get_note_script_hash should return script hash
let code = "
use.kernel::prologue
use.kernel::note->note_internal
use.kernel::note

begin
exec.prologue::prepare_transaction
exec.note_internal::prepare_note
dropw dropw dropw dropw
exec.note::get_note_script_hash

# truncate the stack
swapw dropw
end
Copy link
Contributor

Choose a reason for hiding this comment

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

You said that you used the test for the get_serial_number as a reference, but it turned out that this test has some flaws in it. Your test (and get_serial_number one) could be rewritten like so:

    use.kernel::prologue
    use.miden::note
    begin
        exec.prologue::prepare_transaction
        exec.note::get_note_script_hash

        # truncate the stack
        swapw dropw
    end

So there are three comments:

  1. As far as I know, we should avoid using procedures from the kernel lib directly, so in that case we should use the get_note_script_hash procedure from the miden lib, and not from the kernel.
  2. We don't need to import the same module with different names. Probably at first there were both miden::note and kernel::note modules, that's why we wanted to have kernel::note as an internal one, but for now it is unnecessary.
  3. We don't need to call note::prepare_note procedure — it just moves some data on the stack, without updating any internal variables. We don't use this data, so this execution could be safely removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for the explanation! I fixed this test and the serial number one.

";

let process = tx_context.execute_code(code).unwrap();

let script_hash: Word = tx_context.input_notes().get_note(0).note().script().hash().into();
assert_eq!(process.stack.get_word(0), script_hash);
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 we can avoid importing the Word here by calling the .as_elements() on the RpoDigest returned from the .hash():

let script_hash = tx_context.input_notes().get_note(0).note().script().hash();
assert_eq!(process.stack.get_word(0), script_hash.as_elements());

}
Loading