Create Auto Rebalance Algorithm - #583
Open
markettes wants to merge 24 commits into
Open
Conversation
…e managed node key in methods
…tion and enhance routing state handling
…ate ChannelFeeOptimizerJob to use GetOpenChannels
Add PsbtApprovalValidator gate to ChannelOperationRequestPSBT and WalletWithdrawalRequestPSBT repositories to reject malformed or duplicate human signatures before they reach the signing path. - Wire ValidateApproval into AddAsync/AddRangeAsync for channel operation PSBTs, using SIGHASH_NONE to match ChannelRequests.razor. - Server-generated rows (template, internal wallet, finalised) bypass validation; human approvals are checked against the request's template PSBT and existing signatures. - Include ChannelOperationRequestPsbts when loading the request so the validator has the full signature set. - Log rejections with the request id and reason, and surface the error to the caller. - Add BitcoinService tests covering the duplicate-template-row and related rejection scenarios on the withdrawal signing path.
… engine (#559) * feat: add GetInfo and GetBlockHeight methods to LightningClientService and LightningService * feat(migrations): add routing engine foundation with new columns and tables * feat: implement repositories for channel fee, flow analytics, and routing states; add corresponding interfaces and tests * feat: add BlockHeightHelper and ChannelOwnershipHelper with corresponding tests * feat: add PeerCategorizationService with tests for category computation logic * feat: add TargetRatioReevaluationJob for routing engine with scheduling configuration * feat: add heuristic routing engine configuration options for dynamic fee management and automated rebalancing * feat: update ROUTING_ENGINE_DRY_RUN to handle null values in environment variable * feat: enhance SetChannelFeePolicy to support engine-driven fee updates and improve audit logging * feat: update routing engine fee parameters * feat: add methods to retrieve channel fee states by managed node pub key and check in-flight rebalances by source channel * feat: Implement Fee Optimizer Service and Channel Fee Optimization Job * chore: rename test file * feat: migrate channel flow analytics methods to forwarding HTLC event repository and remove unused repository interfaces * feat: update RoutingEngineDryRun default value to false and adjust related documentation * fix: change default value of IsDynamicFeeEnabled to false in Channel model and related migrations * feat: remove PeerCategorizationService from service registrations in Program.cs * fix: remove every mention to phases or development steps * docs: enhance comments in Constants.cs for clarity on routing engine parameters * refactor: remove circuit breaker feature * refactor: remove circuit breaker logic from ChannelFeeOptimizerJob * fix: align ChannelFeeOptimizerJob with PR1 (ForwardingHtlcEventRepository + no dry-run) * refactor: remove ReservedFeeSats from Rebalance model and related migrations * test: clarify comments and update test logic in RebalanceRepositoryRoutingEngineTests * fix: ensure only active channels are processed in TargetRatioReevaluationJob * refactor: remove RebalanceDeadband from FeeOptimizerService and related tests * fix: remove restore fees when auto fee enabled * feat: allow positive inbound fees if node is enabled * feat: check for enabling channel auto fees and blocing changing them if enabled * feat: add methods to retrieve open channels and channels with dynamic fee enabled * refactor: remove unnecessary baseline * refactor: update fee optimizer constants and methods for fee calculations * refactor: simplify retrieval of open channels in TargetRatioReevaluationJob * refactor: improve ChannelFeeOptimizerJob * refactor: remove baseline fee properties from ChannelFeeState and related migrations * refactor: change max outbound ppm back to 5000 * feat: remove ChannelFeeState when channel or node is disabled from the fee engine * fix: change comments to reflect the behavior * fix: remove unnecessary repository * fix: update comment to include ChannelFeeOptimizerJob in job cadence description * refactor: optimize channel lookup by using dictionary for channels in ChannelFeeOptimizerJob and TargetRatioReevaluationJob * feat(migrations): add routing engine foundation with new columns and tables * feat: implement repositories for channel fee, flow analytics, and routing states; add corresponding interfaces and tests * feat: add PeerCategorizationService with tests for category computation logic * feat: update ROUTING_ENGINE_DRY_RUN to handle null values in environment variable * feat: enhance SetChannelFeePolicy to support engine-driven fee updates and improve audit logging * feat: update routing engine fee parameters * feat: add methods to retrieve channel fee states by managed node pub key and check in-flight rebalances by source channel * feat: Implement Fee Optimizer Service and Channel Fee Optimization Job * feat: migrate channel flow analytics methods to forwarding HTLC event repository and remove unused repository interfaces * feat: remove PeerCategorizationService from service registrations in Program.cs * fix: remove every mention to phases or development steps * refactor: remove circuit breaker logic from ChannelFeeOptimizerJob * fix: align ChannelFeeOptimizerJob with PR1 (ForwardingHtlcEventRepository + no dry-run) * refactor: remove RebalanceDeadband from FeeOptimizerService and related tests * refactor: remove ReservedFeeSats from Rebalance model and related migrations * feat: add methods to retrieve open channels and channels with dynamic fee enabled * refactor: remove unnecessary baseline * refactor: update fee optimizer constants and methods for fee calculations * refactor: simplify retrieval of open channels in TargetRatioReevaluationJob * refactor: improve ChannelFeeOptimizerJob * refactor: change max outbound ppm back to 5000 * feat: remove ChannelFeeState when channel or node is disabled from the fee engine * fix: change comments to reflect the behavior * fix: remove unnecessary repository * fix: update comment to include ChannelFeeOptimizerJob in job cadence description * refactor: optimize channel lookup by using dictionary for channels in ChannelFeeOptimizerJob and TargetRatioReevaluationJob * feat: add PeerCategorizationService and adjust job scheduling interval * feat: add E2ETestBase for shared plumbing in end-to-end tests * feat: add TargetRatioReevaluationJobTests to validate channel categorization logic * refactor: replace service interfaces with static classes for FeeOptimizer and PeerCategorization * feat: update routing engine fee parameters and introduce new thresholds for better fee management * feat: rename GetChannelsFeeEngine to GetChannelsByOpenAndDynamicFeeEnabled for clarity * chore: change log level from Debug to Information for channel action logging * refactor: simplify documentation for ComputeNextPolicy parameters in FeeOptimizerService * refactor: update DeleteByChannelId method to return boolean instead of int for clarity * feat(e2e): Implement fee engine flow tests and enhance routing job scheduling * fix: Refactor fee engine tests and remove unused PriorityOrderer class * refactor: reusing base methods and remove unused methods from test * docs: Update README.md for e2e test structure and execution details * fix: Enhance error handling for already connected peers in LndTestClient * fix: update database connection string variable name * feat: add minimum channel size configuration for routing engine
* Scope the coin-selection ignore list to the queried wallet GetIgnoredOutpoints told the NBXplorer backend to skip every locked and frozen outpoint in the database, not just the ones belonging to the wallet being queried: GetLockedUTXOs is called with both filters null, and the frozen lookup is a plain scan of the UTXOTags table with no wallet join. Outpoints from other wallets can never appear in the response anyway, so they were pure weight on the request line, and enough of them pushed it past Kestrel's 8KB limit for a 414 that silently knocked the whole selection back to the plain UTXO listing, discarding the strategy, limit and amount. Intersect the combined list with the wallet's own UTXO set, which the caller has already fetched, and dedupe it: an outpoint that was both locked and frozen was being sent twice. Dust comes off the list entirely. The backend already drops it by value, so enumerating it spent one query parameter per dust UTXO to say something a single parameter says better. FilterLockedFrozenUTXOs still strips dust from the response, so nothing dusty can reach the caller regardless. The gRPC path in NodeGuardService builds its own, already wallet-scoped copy of this list and is deliberately left alone here. * POST the selectutxos ignore list instead of putting it on the request line Scoping the list to the wallet shortens it but does not bound it: a wallet with enough locked or frozen UTXOs of its own still overflows Kestrel's default 8192-byte MaxRequestLineSize and earns a 414, because every ignored outpoint costs another repeated query parameter at roughly 83 bytes apiece. Send the outpoints in a JSON body instead. The selection parameters stay on the query string. Kestrel's body limit is 30,000,000 bytes against 8,192 for the request line, about 3,662x the room, so the list no longer has a practical ceiling — roughly 430,000 outpoints rather than ninety. This makes an NBXplorer serving POST /selectutxos (Elenpay/NBXplorer#18) a hard requirement. Against an older build every call answers 405, which the callers swallow the same way they swallow the 414, so coin selection degrades for every wallet until that image is deployed. Ship and roll out #18 first. Also send minimumValue, so the backend filters dust by value instead of the caller listing each dust outpoint. It defaults to the same 546 the backend hardcoded before, so this is a no-op at the default MINIMUM_UTXO_VALUE_SATS. The tests pin the wire format the fork's controller binds to, at both a handful of outpoints and well past the old ceiling. NBXplorerService had no test at all before this, and the test project had no HttpMessageHandler mocking; Moq covers it without a new package. TestEnvironment supplies NBXPLORER_URI from a module initializer because Constants reads it in a static constructor into a readonly field, so a test cannot set it after the fact, and ExplorerClient construction throws on a null URI. A value already set by the environment wins, so CI can point it elsewhere.
…#576) * Scope coin-selection ignore list to queried wallet (#575) * Scope the coin-selection ignore list to the queried wallet GetIgnoredOutpoints told the NBXplorer backend to skip every locked and frozen outpoint in the database, not just the ones belonging to the wallet being queried: GetLockedUTXOs is called with both filters null, and the frozen lookup is a plain scan of the UTXOTags table with no wallet join. Outpoints from other wallets can never appear in the response anyway, so they were pure weight on the request line, and enough of them pushed it past Kestrel's 8KB limit for a 414 that silently knocked the whole selection back to the plain UTXO listing, discarding the strategy, limit and amount. Intersect the combined list with the wallet's own UTXO set, which the caller has already fetched, and dedupe it: an outpoint that was both locked and frozen was being sent twice. Dust comes off the list entirely. The backend already drops it by value, so enumerating it spent one query parameter per dust UTXO to say something a single parameter says better. FilterLockedFrozenUTXOs still strips dust from the response, so nothing dusty can reach the caller regardless. The gRPC path in NodeGuardService builds its own, already wallet-scoped copy of this list and is deliberately left alone here. * POST the selectutxos ignore list instead of putting it on the request line Scoping the list to the wallet shortens it but does not bound it: a wallet with enough locked or frozen UTXOs of its own still overflows Kestrel's default 8192-byte MaxRequestLineSize and earns a 414, because every ignored outpoint costs another repeated query parameter at roughly 83 bytes apiece. Send the outpoints in a JSON body instead. The selection parameters stay on the query string. Kestrel's body limit is 30,000,000 bytes against 8,192 for the request line, about 3,662x the room, so the list no longer has a practical ceiling — roughly 430,000 outpoints rather than ninety. This makes an NBXplorer serving POST /selectutxos (Elenpay/NBXplorer#18) a hard requirement. Against an older build every call answers 405, which the callers swallow the same way they swallow the 414, so coin selection degrades for every wallet until that image is deployed. Ship and roll out #18 first. Also send minimumValue, so the backend filters dust by value instead of the caller listing each dust outpoint. It defaults to the same 546 the backend hardcoded before, so this is a no-op at the default MINIMUM_UTXO_VALUE_SATS. The tests pin the wire format the fork's controller binds to, at both a handful of outpoints and well past the old ceiling. NBXplorerService had no test at all before this, and the test project had no HttpMessageHandler mocking; Moq covers it without a new package. TestEnvironment supplies NBXPLORER_URI from a module initializer because Constants reads it in a static constructor into a readonly field, so a test cannot set it after the fact, and ExplorerClient construction throws on a null URI. A value already set by the environment wins, so CI can point it elsewhere. * E2E: coin selection with an ignore list too long for the request line Guards the transport switch end to end. The test funds a wallet with 130 small outputs plus a few larger ones in a single transaction, freezes only the small ones through the AddTags RPC, and asserts GetAvailableUtxos still returns the wallet's real UTXOs and never surfaces a frozen one. Frozen UTXOs rather than dust carry the list on purpose. Dust is excluded by value now, so a dust-driven test would stay green even if the exclusion list never reached the server; frozen UTXOs above the dust floor have no other reason to be missing. The test also thaws one at the end and asserts it comes back, which pins their absence on the tag rather than on anything incidental. Before the fix this is a 414 that GetAvailableUtxos swallows into an empty UTXOChanges, so the RPC reports success with nothing selectable for a 20 BTC wallet. That is the assertion that flips. Uses the fourth seeded wallet, not the hot wallet the rest of the suite withdraws from, because the frozen outputs stay in it for the rest of the run. The suite needs an NBXplorer serving POST selectutxos, so the runner gets an NBXPLORER_URI and the test probes the route first, failing with that diagnosis rather than looking like a coin-selection bug. Without the probe it would be red identically before and after the fix, which is no test at all. TODO before this is merged: repin the nbxplorer image to the immutable tag of a build carrying the POST route. See the comment in docker/docker-compose.dev.yml.
…-without-reason Fix sweep node wallets job to return errors without clear reason
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds automated channel rebalancing to the routing engine:
RebalanceInitiatorService(pure,unit-tested) sorts channels into drainable sources and depleted destination peers and pairs them
into sized, profit-gated plans, which
AutoRebalanceJobdispatches throughRebalanceServiceonits own 10-minute cadence.
Every plan is bounded before it goes out — per-channel
IsAutoRebalanceEnabledopt-in, a deadbandeither side of each channel's target ratio, a fee ceiling of
CostToEarnRatio × the destination peer's earn rate, and per-node budget / in-flight / per-run caps — plus per-node dry-run and theglobal
ROUTING_ENGINE_ENABLEDkill switch, which is off by default, so merging this changes nobehaviour until it's switched on.
Supporting changes:
ChannelRoutingStatebecomes per (channel, managed node) so both ends of achannel NodeGuard manages actuate independently; the new
RoutingEngineSnapshotServicebuilds theper-node channel view now shared with
ChannelFeeOptimizerJob; two migrations and unit tests forthe planner and the job.