fix: return DecodedReturn instead of any[] #1540
Conversation
LHerskind
force-pushed
the
lh/single-decoded-return
branch
from
3be7089 to
080415d
Compare
| returnValues.push(this.decodeReturn(this.abi.returnTypes[i])); | ||
| public decode(): DecodedReturn { | ||
| if (this.abi.returnTypes.length > 1) { | ||
| throw new Error('Multiple return values not supported'); |
There was a problem hiding this comment.
shouldn't this be different return types from one function is not supported since you are checking returnTypes.length?
There was a problem hiding this comment.
Update - add a comment how an array is not [Field, Field] but [array{size:2, type:field}]
There was a problem hiding this comment.
Overall looks good to me, however I have concerns over how this will collide with my current pr #1515.
In this PR i alter public function's return values by pushing them into the return_values object in the kernel. I'm overall not sure what the long term solution will be in regards to this as soon as we move public execution to the VM model.
You should be good to merge this, I think it will be my work that needs to change
|
Maybe we can get this into the typechain generator as well, so we get typed return types? I can tackle it after this PR is merged. |
LHerskind
force-pushed
the
lh/single-decoded-return
branch
from
25b97f1 to
8b1f1e9
Compare
Return a single
DecodedReturninstead of theany[]that we are currently returning when as the return values in execution results. As noir let you have one return value (that return value can be an array or struct etc) we should just return that directly instead of wrapping it inside an array for the return values. This makes it more straight-forward to use view functions as well as you can get structs directly that way (example from lending contract)Fixes a naming from
interestAccumulator->interest_accumulatoras well.Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.