add ix_path to InstructionUpdate #180
Conversation
crates/core/src/instruction.rs
Outdated
| pub inner: Vec<InstructionUpdate>, | ||
| /// The index of this instruction within the transaction. | ||
| /// always set after parsing. | ||
| pub ix_path: Option<IxPath>, |
There was a problem hiding this comment.
ig we can call is ix_index , wyt @kespinola
| pub ix_path: Option<IxPath>, | |
| pub ix_index: Option<IxIndex>, |
There was a problem hiding this comment.
IxIndex sounds nice but does it imply that the thing is multiple levels deep? I'm good with ix_index nevertheless
There was a problem hiding this comment.
Path is good with me I'd just call the field path though since its on an instruction update object already.
crates/core/src/instruction.rs
Outdated
| .map(|i| Self::parse_one(Arc::clone(&shared), i)) | ||
| .collect::<Result<Vec<_>, _>>()?; | ||
|
|
||
| for (index_outer, outer_instr) in outer.iter_mut().enumerate() { |
There was a problem hiding this comment.
| for (index_outer, outer_instr) in outer.iter_mut().enumerate() { | |
| for (index_outer, outer_ix) in outer.iter_mut().enumerate() { |
crates/core/src/instruction.rs
Outdated
|
|
||
| Self::parse_inner(&shared, inner_instructions, &mut outer)?; | ||
|
|
||
| for outer_instr in &outer { |
There was a problem hiding this comment.
same comment as above
crates/core/src/instruction.rs
Outdated
| pub inner: Vec<InstructionUpdate>, | ||
| /// The index of this instruction within the transaction. | ||
| /// always set after parsing. | ||
| pub ix_path: Option<IxPath>, |
There was a problem hiding this comment.
any reason this is optional ?
There was a problem hiding this comment.
my approach visits the tree after it got constructed and updates the path
There was a problem hiding this comment.
Is there anyway to compute it when the InstructionUpdate is being formed so its not an option?
There was a problem hiding this comment.
hm thought a bit about that but it that's not possible without modifying the traversal strategy
There was a problem hiding this comment.
I can try to change parse_inner to do traversal before the actual mapping (parse_one_inner)
There was a problem hiding this comment.
ig I found an easy way to derive the path from the stack height array
one more question: should we log the path in the places where errors/logs in vixen are referring to an instruction?
| pub struct IxIndex(Vec<u32>); | ||
|
|
||
| impl IxIndex { |
There was a problem hiding this comment.
You are already in instruction module so call perhaps call it path so its instruction::Path
3 participants
Addresses this issue
Test with filtered-pipeline