-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: add expiration
transaction policy
#1583
base: master
Are you sure you want to change the base?
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.
Neat -- great tests!
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.
Left a remark about our general approach to testing.
PR looks good
@@ -248,6 +260,8 @@ pub trait Transaction: | |||
|
|||
fn with_maturity(self, maturity: u32) -> Self; | |||
|
|||
fn expiration(&self) -> Option<u64>; |
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.
should we a setter with_expiration
here, like we have for maturity
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.
Good question. Well other policies are also missing the with_
methods. We can either remove it or add them for all? What do you think?
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.
with_maturity
changes the tx_id
so I would like to remove it.
@@ -1245,3 +1245,80 @@ async fn predicate_configurables_in_blobs() -> Result<()> { | |||
|
|||
Ok(()) | |||
} | |||
|
|||
#[tokio::test] | |||
async fn predicate_transfer_respects_maturity_and_expiration() -> Result<()> { |
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.
At some point, we should discuss how we want to approach testing for features like this, where we cover various transaction and input types. It feels like the level of granularity here (and in other cases) might be higher than what we actually benefit from.
closes: #1577
Release notes
Added the
expiration
transaction policy. Now the user can limit until which block height the transaction is valid.Breaking Changes
expiration: Option<u64>
field toTxPolicies
structTransaction
trait methodmaturity
now returnsOption<u64>
Transaction
trait methodwith_maturity
is removed.Checklist