Skip to content

Conversation

@positiveblue
Copy link
Contributor

Bring back the sponsor a batch feature.

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 autorenew batch version, as long as the batch confirmation does not trigger an expiry extension.

Added some tests to the RenewAccount RPC 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.

@positiveblue positiveblue changed the title Sponsor batch Enable batch participants to sponsor the batch by renewing their account Dec 10, 2021
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.
@lightninglabs-deploy
Copy link
Collaborator

@bhandras: review reminder
@guggero: review reminder

Copy link
Contributor

@guggero guggero left a 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
Copy link
Contributor

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 {
Copy link
Contributor

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
Copy link
Contributor

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() {
Copy link
Contributor

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...

Copy link
Contributor Author

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

@guggero
Copy link
Contributor

guggero commented Dec 14, 2021

!lightninglabs-deploy mute 2022-Feb-01

@guggero guggero deleted the sponsor-batch branch December 20, 2021 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants