-
Notifications
You must be signed in to change notification settings - Fork 47
Enable batch participants to sponsor the batch by renewing their account #323
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
Conversation
The `mockgen` tool seems to have a bug and it is not able to parse a valid go file. I had to change the `[sha256.Size]byte` to `[size]byte` and define `size` in the order package in the same way it is defined in the sha256 one.
The Marshaler interface is used to transform internal types to decorated RPC ones. It decouples that functionality form the rpcServer so it is easier to test exhaustively.
If an account is currently participating in a batch that has not been confirmed yet and the account expiry will not change after the batch is confirmed (account autorenwal) let the user renew it's account to perform a CPFP.
9831589 to
7924306
Compare
guggero
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.
Nice update to the mocks and unit tests! Great to already see them in action for every code addition 🎉
I think we need to discuss the batch sponsoring again, it's possible I misunderstood the problem when we added the original restriction. Unfortunately we don't currently have an easy way to find out if the account was extended in the last batch, at least not on the client side.
| type marshalerConfig struct { | ||
| // GetOrders returns all orders that are currently known to the store. | ||
| GetOrders func() ([]order.Order, error) | ||
| // Terms returns the current dynamic auctioneer terms like max account |
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.
nit: add empty line between elements.
|
|
||
| // marshalerConfig contains all of the marshaler's dependencies in order to | ||
| // carry out its duties. | ||
| type marshalerConfig struct { |
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.
nit: move to own file (e.g. marshaler.go)?
| @@ -0,0 +1,16 @@ | |||
| package pool | |||
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.
nit: typo in commit message.
| // hasPendingExtension returns if the accountKey is waiting for a batch to be | ||
| // confirmed to extend the expiry height. | ||
| func (s *rpcServer) hasPendingExtension(accountKey *btcec.PublicKey) bool { | ||
| if s.orderManager.HasPendingBatch() { |
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.
Unfortunately this won't really work. The orderManager.HasPendingBatch() only returns true while a current batch is ongoing and hasn't been fully signed yet. As soon as the final batch TX is published this will be empty again.
I'm trying to remember, what was the original reason we didn't allow an account extension to be issued by the user if the account was still in the state StatePendingBatch? Because it could potentially throw the account out of sync with the server if another update TX was added?
Maybe I gave you some incorrect info there...
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.
😞 thanks for catching this one
|
!lightninglabs-deploy mute 2022-Feb-01 |
Bring back the
sponsor a batchfeature.Accounts participating in a batch will be able to sponsor a batch by renewing their account. That applies to all the accounts, included the ones with the
autorenewbatch version, as long as the batch confirmation does not trigger an expiry extension.Added some tests to the
RenewAccountRPC endpoint. I created a couple of interfaces (Account and Order Managers) to decouple a bit the logic and use mocks for the tests. Same applies to the account rpc marshaler.