allow specification of 'wallet' for ensure funds calls #129
Conversation
| @@ -209,7 +209,7 @@ func (c *Client) Start(ctx context.Context, p ClientDealProposal) (cid.Cid, erro | |||
| ClientCollateral: big.Zero(), | |||
| } | |||
|
|
|||
| if err := c.node.EnsureFunds(ctx, p.Client, dealProposal.ClientBalanceRequirement()); err != nil { | |||
| if err := c.node.EnsureFunds(ctx, p.Client, p.Client, dealProposal.ClientBalanceRequirement()); err != nil { | |||
There was a problem hiding this comment.
Just curious what is the value of StorageClientNode taking two parameters if it's always going to be the same (it's clear why for the provider where the values are different)
There was a problem hiding this comment.
ah cause we have a common method in the integration test... I wonder if it would be better to just rewrite the test to not use fake common for that method.
There was a problem hiding this comment.
cause in the implementation it's actually two different entire classes that don't share anything
There was a problem hiding this comment.
(as mentioned in slack) lotus also uses a common thingy for implementing this. We could have them be separate interfaces, but i think the mental overhead is lower if we just have it be the same. Plus, theres no reason you couldnt fund your deal collateral with a different account
When trying to ensure funds for a miner, we were just adding funds to the worker key, which doesnt really help much (its not associated with the miner on chain in that way). Switching that over, things then tried to send money from the miner actor, which obviously doesnt work, so now we pass the address you want to ensure has funds on the market, and the address to send funds from.