-
Notifications
You must be signed in to change notification settings - Fork 125
add_signature_or_message method for ProposedBundle #258
Conversation
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.
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
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
ProposedBundle
object.Motivation
Ability to easily add custom messages to transactions after
ProposedBundle
creation.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.