feat(multiclient): adds timeouts to retryWithBackups and dialWithRetry

[giogam]

This PR enhances the MultiClient implementation by adding timeout support and improving test coverage.

Timeout Handling:

• Added Timeout and DialTimeout to RetryConfig, with defaults for RPC and dial operations.
• Updated retryWithBackups and dialWithRetry to use context.WithTimeout, enforcing timeouts for RPC calls and connections.

Context Propagation:

• All MultiClient methods (SendTransaction, CallContract, etc.) now pass context.Context through to support cancellation and timeouts consistently.

Test Improvements:

• Added TestMultiClient_dialWithRetry to cover dial timeouts and failure scenarios.
• Expanded TestMultiClient_retryWithBackups to simulate timeouts and long-running operations using context.

[changeset-bot]

🦋 Changeset detected

Latest commit: 2b3b0c8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
chainlink-deployments-framework	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

some changes

[graham-chainlink]

So we are exposing context for each public method in this PR, which means user can pass down their own context with timeout, but then inside each method we override it with the RetryConfig timeout value, should we honour their timeout too or not?

I would expect that if a method accepts context , it should uses it like DialContext in postgres.

A thought is that if we really want to enforce our own timeout, maybe we could set out own timeout if there are no existing timeout set in the user pass in context? Not super sure, lets see what everyone else thinks.

func WithTimeoutIfNone(parent context.Context, timeout time.Duration) (context.Context, context.CancelFunc) {
    // Check if the parent context already has a deadline
    if _, hasDeadline := parent.Deadline(); hasDeadline {
        // Return the original context and a no-op cancel function
        return parent, func() {}
    }

    // No deadline: apply timeout
    return context.WithTimeout(parent, timeout)
}

// usage

ctx, cancel := WithTimeoutIfNone(ctx, DEFAULT_TIMEOUT)

Also with WaitMined, then now there is some inconsistency, a new user could set timeout on the RetryConfig but then doent know that it doesnt affect WaitMined but it affect the rest, a surprise?

[giogam]

So we are exposing context for each public method in this PR, which means user can pass down their own context with timeout, but then inside each method we override it with the RetryConfig timeout value, should we honour their timeout too or not?

Good question. I looked into the ethclient implementation and contract bindings, and it does appear possible to inject a context from a changeset. The contract bindings accept context via TransactOpts, see here and here.
The ethclient implementation handles this in a similar way to what you suggested, for example, here. I’d say it’s unlikely someone is doing this, but it’s definitely possible. I’m happy to update the code with your suggestion, it’s a small change anyway.

Also with WaitMined, then now there is some inconsistency, a new user could set timeout on the RetryConfig but then doent know that it doesnt affect WaitMined but it affect the rest, a surprise?

For WaitMined, I think it makes sense to have a separate, and potentially longer, timeout, since transactions can sometimes take a while to get mined. We could improve the code to make this configurable, but I’d still keep it as a separate setting from the client retry config.

[graham-chainlink]

For WaitMined, I think it makes sense to have a separate, and potentially longer, timeout, since transactions can sometimes take a while to get mined. We could improve the code to make this configurable, but I’d still keep it as a separate setting from the client retry config.

Sounds good! lets make sure we comment it so users know as well