Skip to content
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

chain: add mempool lookup for outpoints #910

Merged
merged 4 commits into from
Mar 5, 2024

Conversation

yyforyongyu
Copy link
Collaborator

This PR adds a new method LookupInputMempoolSpend to directly check whether a given input has been spent or not in the mempool.

chain/bitcoind_events_test.go Outdated Show resolved Hide resolved
chain/bitcoind_events_test.go Show resolved Hide resolved
@saubyk
Copy link

saubyk commented Feb 29, 2024

cc: @guggero @Crypt-iQ for review please

@guggero guggero self-requested a review February 29, 2024 15:26
Copy link
Collaborator

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

utACK, LGTM 🎉

chain/btcd.go Outdated
// disconnected.
func NewRPCClientWithConfig(cfg *RPCClientConfig) (*RPCClient, error) {
// Make sure the config is supplied.
if cfg == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the way Go works, this check could be in the validate() method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't that cause a nil deference? Or you mean before initializing the client we do a validation on config in lnd?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the thing is, in Go you can call a method on a struct, even if the struct itself is nil.
So you can do:

// validate checks the required config options are set.
func (r *RPCClientConfig) validate() error {
	// Make sure the config is supplied.
	if r == nil {
		return errors.New("missing rpc config")
	}

	// Make sure retry attempts is positive.
	if r.ReconnectAttempts < 0 {
		return errors.New("reconnectAttempts must be positive")
	}
	...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh cool, TILed, thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh at time a check on the nil pointer can actually be useful.

@Roasbeef
Copy link
Member

Roasbeef commented Mar 1, 2024

Can be rebased with the btcd version merged!

This commit adds a new method to init the RPC client. Unlike the
`NewRPCClient`, it now takes a config to allow users specifying
notification handlers.

In the future, `NewRPCClient` should be replaced by this method. For now
we implement it this way so other callsites relying on this method can
stay out of being affected.
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎢

@Roasbeef Roasbeef merged commit f7c216e into btcsuite:master Mar 5, 2024
3 checks passed
@yyforyongyu yyforyongyu deleted the add-mempool-lookup branch March 5, 2024 11:33
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.

None yet

5 participants