-
Notifications
You must be signed in to change notification settings - Fork 126
add_signature_or_message method for ProposedBundle #258
add_signature_or_message method for ProposedBundle #258
Conversation
todofixthis
left a comment
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.
Recommending that this method not be callable after the bundle is finalised. Otherwise looks good w/ suggestions+questions.
todofixthis
left a comment
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.
Recommending that this method not be callable after the bundle is finalised. Otherwise looks good w/ suggestions+questions.
d8843bf to
28f0586
Compare
|
Thanks for the review @todofixthis, I implemented your suggested 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.
LGTM! A couple of fragements still managed to sneak in; otherwise looks great!
The method can insert custom messages or signatures into transactions in a ProposedBundle object. Must be called before bundle is finalized.
28f0586 to
22b4f46
Compare
todofixthis
left a comment
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.
LGTM! 🎉
Solves #253
Description
The method can insert custom messages or signatures into transactions in a
ProposedBundleobject.Motivation
Ability to easily add custom messages to transactions after
ProposedBundlecreation.Additional Info
The test file is formatted with 2 space indentation to make review easier. In a future PR old test files will be converted to use 4 space indentation as per PEP-8.