[4/x][programmable transactions] Added MoveCall execution #8406
Conversation
tnowacki
commented
Feb 18, 2023
tnowacki
commented
- Basically had to rewrite the Move Adapter. I wouldn't worry about the lack of code sharing as I think the current adapter can die once we migrate everything.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
I will do some commenting later :) |
- Basically had to rewrite the Move Adapter. I wouldn't worry about the lack of code sharing as I think the current adapter can die once we migrate everything.
| V::try_from_value(val) | ||
| } | ||
|
|
||
| pub fn copy_arg<V: TryFromValue>( |
There was a problem hiding this comment.
I may be misunderstanding what this function is supposed to do, but I would have expected a check here that the arg was copyable (i.e. a primitive type, or has the copy ability).
There was a problem hiding this comment.
I did not do that because we will have to check the signature, and I wanted to do that before giving an ability error. For example you call
foo(x: u64)
But with a Coin as the argument. I want to give a type error, not some sort of "invalid use of copy" error.
In other words, "copy" here is more about cloning the value from a rust semantics standpoint. Maybe that would be a better name? clone_arg?
| } | ||
| } | ||
|
|
||
| pub fn push_command_results(&mut self, results: Vec<Value>) -> Result<(), ExecutionError> { |
There was a problem hiding this comment.
Is it worth introducing the concept of a "Frame", that is scoped per-command, to track borrows, and hold results while they are built?
There was a problem hiding this comment.
I think it totally would be if borrows weren't ensured to be empty at the end of each command. But I might not be fully understanding the suggestion. Further thoughts?
There was a problem hiding this comment.
One of the responsibilities of Frame would be to track the borrows and purge them at the end of each command.
| let layout = MoveTypeLayout::Struct(MoveStructLayout::Runtime(vec![MoveTypeLayout::Vector( | ||
| Box::new(MoveTypeLayout::U8), | ||
| )])); |
There was a problem hiding this comment.
I'm kind of surprised that this layout does not already exist in a more general place -- is it worth putting somewhere more general for the next person that needs to use it?
There was a problem hiding this comment.
It exists/is hard coded in the adapter. But might be worthwhile having it somewhere... just not sure where
| // Returns None for all other types | ||
| pub fn is_tx_context< | ||
| E: fmt::Debug, | ||
| S: ResourceResolver<Error = E> |
There was a problem hiding this comment.
I feel like these trait constraints pop-up so often that they deserve an alias.
There was a problem hiding this comment.
Can you do that???
There was a problem hiding this comment.
There is no trait alias, but I think traits support some degree of duck typing? So I added a marker trait and we will see if that works.
EDIT: Added some other magic... oh traits
| Type::Vector(inner) => stack.push(&**inner), | ||
| Type::Struct(idx) => { | ||
| let Some(s) = context.session.get_struct_type(*idx) else { | ||
| invariant_violation!("Loaded struct unreachable") |
There was a problem hiding this comment.
This error is a little confusing to me -- what does "unreachable" mean in the context?
|
cc @dariorussi who had be interested in this work |
