-
Notifications
You must be signed in to change notification settings - Fork 249
feat(ymax-planner): Calculate steps by linear programming over a model of the blockchain network #11932
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
Deploying agoric-sdk with
|
Latest commit: |
3732799
|
Status: | ✅ Deploy successful! |
Preview URL: | https://53a8ba01.agoric-sdk.pages.dev |
Branch Preview URL: | https://dt-solver-graph.agoric-sdk.pages.dev |
|
||
Node (vertex) types (all implement `AssetPlaceRef`): | ||
- Chain hubs: `@ChainName` (e.g. `@Arbitrum`, `@Avalanche`, `@Ethereum`, `@noble`, `@agoric`). A single hub per chain collects and redistributes flow for that chain. | ||
- Protocol / pool leaves: `Protocol_Chain` identifiers (e.g. `Aave_Arbitrum`, `Beefy_re7_Avalanche`, `Compound_Ethereum`). Each is attached to exactly one hub (its chain). |
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.
Since there is, presumably, a single edge for each pool, we can do away with them altogether and solve the flow on a network on only the hubs.
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.
There may be more than one. Those edges may have fees or time impacts. There also may in the future not be just a single edge (we could have USDN to USDNVault as well as @noble to USDNVault, or direct transfers between Beefy vaults without coming out to the base account).
We build an LP/MIP object with: | ||
- `variables`: one per flow variable `f_edgeId`, each holding coefficients into every node constraint and capacity constraint; plus binary usage vars `y_edgeId` when required. | ||
- `constraints`: | ||
- Node equality constraints (one per node with any incident edges). |
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.
No sure if you have already tried but solving this could be, at times, really hard (this being an NP-Hard problem) even on reasonable small graphs. Not sure if it is advised to do it live. I would at least store the solutions to matched and fetched for future possibly by an LLM.
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.
From ChatGPT analysis, based on our usage of Mixed Integer Linear Program solving.
Practical Complexity for Our Usage:
Expected case: O(k × n^3) where k is typically much smaller than 2^25
Performance Characteristics:
- Variables: ~50 (25 integer + 25 binary)
- Constraints: ~63
- Expected solve time: Milliseconds to low seconds for typical rebalancing scenarios
- Memory usage: Linear in problem size (~O(m×n))
Why it performs better than worst-case:
- Network flow structure: Our problem has special structure (min-cost flow with binary activation) that branch-and-bound handles efficiently
- Small scale: 25 binary variables is manageable for modern MIP solvers
- Good LP relaxation bounds: The continuous relaxation likely provides tight bounds, reducing the branch-and-bound tree size
- Practical branching: Most edges will have clear economic preferences, leading to early pruning
Practical scaling: Should handle networks with 50-100 edges reasonably well
5f41cb9
to
e512eae
Compare
}); | ||
t.deepEqual(steps, [ | ||
{ src: '<Deposit>', dest: '@agoric', amount: token(100n) }, | ||
// TODO dckc should this go through +agoric? |
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.
I think the test case at line 142 will suffice and we don't need this test case anymore- the contract will handle moving the tokens from <Deposit>
to +agoric
[A]: token(50n), | ||
[B]: token(30n), | ||
[C]: token(20n), | ||
'@Arbitrum': ZERO, |
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.
Interface question: can we omit targets with ZEROs?
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.
No. If there is not a target, that means "leave it the same" rather than "use those funds for the operation".
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.
don't turn off tracing; it includes critical operational logging
// XXX Define in AmountMath? | ||
const compareAmounts = (a: NatAmount, b: NatAmount): -1 | 0 | 1 => { | ||
const aValue = a.value; | ||
const bValue = b.value; | ||
// eslint-disable-next-line no-nested-ternary | ||
return aValue > bValue ? 1 : aValue < bValue ? 1 : 0; | ||
}; |
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.
why not use AmountMath.isGTE
? why ignore brands?
const current = balances({ [A]: 100n, [B]: 0n, [C]: 0n }); | ||
const targetBps = { [A]: 3400n, [B]: 3300n, [C]: 3300n }; | ||
const { steps } = await planRebalanceFlow({ | ||
mode: 'cheapest', |
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.
I think all these tests specify "cheapest", but the default mode is "fastest". I think we mostly want to run in "fastest", so we should add tests for that mode.
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.
Added a "solver differentiates cheapest vs. fastest" test to packages/portfolio-contract/test/rebalance.test.ts, currently failing.
dest: 'USDN', | ||
detail: { | ||
usdnOut: 425n, | ||
usdnOut: 430n, |
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.
NOTE These usdnOut
number differences are because the code hard-coded 1% fee, whereas the actual USDN fee is 5 basis points.
3dc80f3
to
64320f8
Compare
- add feeMode to arcs in the graph - switch on feeMode when making steps to add fee info - update all networks definitions and tests - plumb feeBrand through graph building NOTE computed fee work will need to propagate from portfolio-actors to plan-solve.
(differentiated by transfer protocol)
…astest", currently failing
da9f98b
to
42b0a4f
Compare
42b0a4f
to
4feda74
Compare
async (t, mode) => { | ||
const current = balances({ [A]: 50n, [B]: 30n, '<Cash>': 0n }); | ||
const { steps } = await planRebalanceFlow({ | ||
mode: mode as RebalanceMode, |
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.
why isn't mode already correctly typed?
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.
No idea. It's test code and I didn't want to waste more time on it.
This pull request has been removed from the queue for the following reason: The merge conditions cannot be satisfied due to failing checks: You may have to fix your CI before adding the pull request to the queue again. |
@Mergifyio requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
The approach:
javascript-lp-solver
linear programming solverDescription
packages/portfolio-contract/src/network/network.prod.ts represents the blockchain network as a collection of interlinked hubs with attached pools and local "places", including time and fee metadata about the inter- and intra-hub links, which is then integrated with current and target allocations to produce an LP model in which variables represent use of the links and constraints apply to the flow over them (in particular, a net flow constraint for each node whose allocation should change). Once a solution has been identified, the flows are normalized to group activity by hub and apply deterministic ordering within that overall pattern.
Security Considerations
The chosen library is inherently floating-point, and there's a risk that no solution is identified merely because a value that should be integral is off by a small amount (as indeed manifests in the test cases if
jsLPSolver.Solve
is called with the default precision of 1e-9 rather than 1e-6). To mitigate this, the error messages of such failures include the returned negative result.Scaling Considerations
Performance is an explicit concern of our chosen library, and expected to be on the order of 20 ms for the scale of our inputs (given a small-world network of 10–20 [tree-like] hubs with a small number of child pools each: one binary + one integer variable per edge, two or three constraints per edge, and one constraint per node).
Documentation Considerations
Includes some Markdown files that probably ought to be subject to further cleanup.
Testing Considerations
Includes new unit tests.
Upgrade Considerations
n/a