add pull-funds to autopilot #100
Conversation
lanzafame
force-pushed
the
feat/autopilot-pull-funds
branch
from
1c96a38
to
0195576
Compare
There was a problem hiding this comment.
- Line 100 in agent_autopilot.go -
if account == (abigen.Account{}) {- this should beif account.epochsPaid.Cmp(big.NewInt(0)) < 1(if epochsPaid is 0 then their account is reset) - In the
payfunc in agent_pay.go, the 4th argument isdaemon, which seems to just determine if the journal should be closed at the end of the func - why not just close and reopen the journal on each payment instead of passing this argument and determining whether or not the journal should close? Seems like within each cron tick of running the event loop, we should close connections to various things closed to avoid mem leaks?
Good work on this - it's close. Also, lets make sure before we merge this, we adequately test all the payment commands on testnet to make sure things are working how we'd expect
Then we'll get Jim to test it on mainnet to ensure it's working over a few days worth of epochs
| @@ -65,19 +71,21 @@ var agentAutopilotCmd = &cobra.Command{ | |||
| payargs = append(payargs, fmt.Sprintf("%d", amount)) | |||
There was a problem hiding this comment.
Safer to not use string values to pass around a single amount value, and I don't think we have another pay command that ends up using random arguments - you either pass an amount for Principal or Custom or otherwise no amount. Can we simplify this logic to just pass an amount var thats a *big.Int - in the case of to-current, this will just be nil and ignored
There was a problem hiding this comment.
I am going to leave this as is for now and add an issue to resolve this as this is a fairly large change beyond just pull-funds.
cmd/agent_autopilot.go
Outdated
| @@ -151,6 +168,122 @@ var agentAutopilotCmd = &cobra.Command{ | |||
| }, | |||
| } | |||
|
|
|||
| func paymentDue(frequency float64, chainHeadHeight uint64, epochsPaid *big.Int) bool { | |||
There was a problem hiding this comment.
Would be good to get a few test cases in here to make sure logic is correct, especially with frequencies less than 1 day
lanzafame
force-pushed
the
feat/autopilot-pull-funds
branch
from
0195576
to
f940f86
Compare
Also, some extra logging on the transactions, and quoting fixes for the config files.
|
I've done some testing against the localnet ... private testing repo is here: https://github.com/glif-confidential/pools-cli/tree/jim/autopilot-testing I was able to configure it and observe it pull funds when it needed to (with the pull-amount-factor set to 3). I also tried draining all the funds from the miner. If you request to pull more than the available funds from the miner, it will pull the maximum available instead of failing, leaving the miner with zero available funds. This seems like a reasonable behaviour. If users desire a different behaviour, we could implement additional settings. I'll merge this to the |
No description provided.