-
Notifications
You must be signed in to change notification settings - Fork 58
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: Change TransactionArgs
to use AdviceInputs
#793
Conversation
Is this ready for review? Or should I hold off for now? |
I just want to be sure it works with the client and I don't need to add any new methods before I mark it "ready for review". It should be ready in a couple of hours. |
Marking as ready. The client seems to work correctly with these changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you. I left a couple of comments inline which may affect the external interfaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you! I left a couple of small comments inline. After these are addressed, we can merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good! Thank you!
This PR changes the
AdviceMap
insideTransactionArgs
toAdviceInputs
. This is needed so theTransactionArgs
contain aMerkleStore
, which was suggested in this comment frommiden-client
.