remove reward address, clean up mining addr #729
Conversation
|
Still getting a strange error while trying to mine: Which I wasnt getting before rebasing... Will investigate further |
There was a problem hiding this comment.
This looks great from the perspective of someone who deals with the mining code. I've got a comment on waiting to make Mine a member function or at least not investing too much in changing test structures around this new pattern. Will approve after test fixes come through.
As I understand it, now that the miner actor is the recipient of block rewards we need a system for claiming rewards as the miner owner. It might be helpful to see the issue tracking that work linked here.
| @@ -165,7 +162,7 @@ type mineFunc func(ctx context.Context, input Input, nullBlockTimer NullBlockTim | |||
| type NullBlockTimerFunc func() | |||
|
|
|||
| // Mine does the actual work. It's the implementation of worker.mine. | |||
| func Mine(ctx context.Context, input Input, nullBlockTimer NullBlockTimerFunc, blockGenerator BlockGenerator, createPoST DoSomeWorkFunc, outCh chan<- Output) { | |||
| func (w *AsyncWorker) Mine(ctx context.Context, input Input, outCh chan<- Output) { | |||
There was a problem hiding this comment.
I like what you are doing here and just flagging that you can keep the test fixup in an ugly state because the changes coming in #722 will make things nicer. Alternatively we could wait to do this in #722.
As I understand it the reason Mine was a free function hanging around inside w.mine was to facilitate testing by allowing test runs to easily compose the production Start function with a mock Mine function, and to test Mine on its own.
I think fixing tests as things are is doomed to make things ugly because the thing that calls Mine also needs to have a Start method, and without the mine member function workaround I think you'll have to copy paste the Start method on a test worker. There might be more-involved less-ugly workarounds in the context of this PR, but you shouldn't worry about those. In PR #722 I'm separating the Start and Mine behavior to different interfaces which should make creating test instances easier because everything will be talking to an interface that can be swapped for tests.
|
@whyrusleeping it looks like that error message, which I agree needs to be made more informative, is coming from the power table's Unfortunately after doing this |
|
It mines! |
| return nil, err | ||
| } | ||
|
|
||
| return &act, nil | ||
| } | ||
|
|
||
| func hackTransferObject(from, to interface{}) error { | ||
| m := obj.NewMarshaller(cbor.CborAtlas) |
There was a problem hiding this comment.
looks like a local hack, this doesn't work for me
There was a problem hiding this comment.
whoops. That was def a local hack. lemme fix up
53e4b9e to
0a8ca80
Compare
|
things are getting better, but still some tests broken interesting bits I did
|
more tests are passing, less reward address node tests are passing wip - fixing all the tests more tests passing, gengen is getting put to work fix repo and config tests more reward address cleansing
orderbook tests pass
148fa39 to
a60c3cb
Compare
|
getting the following init failure go-filecoin init --genesisfile=/home/kitty/kitty-car/genesis.car --repodir=/home/kitty/filecoins/0
initializing filecoin node at /home/kitty/filecoins/0
Error: malformed stream: invalid appearance of string token; expected start of map |
|
@eefahy which commit did you use to build |
9fe2b5d to
95aa8b6
Compare
95aa8b6 to
58c4083
Compare
core/power_table_view.go
Outdated
| func (v *marketView) HasPower(ctx context.Context, st state.Tree, cstore *hamt.CborIpldStore, mAddr types.Address) bool { | ||
| numBytes, err := v.Miner(ctx, st, cstore, mAddr) | ||
| if err != nil { | ||
| panic(err) //hey guys, dropping errors is BAD |
No description provided.