-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Merge nv16 changes into master #8810
Conversation
- Uses the global MAX_LEN exported by fvm_shared. - Formats errors with debug, for additional information.
chore: deps: update filecoin-ffi
…ad-bundle Cleanup LoadBundle
chore: vm: Rename tracing envvar to LOTUS_VM_ENABLE_TRACING
…id-bundle-cli Fix: cli: clean up manifest-cid-from-car
chore: deps: update to go-libp2p v0.19.4
chore: build: merge v1.15.3 into v1.16.0
fix: build: use the genesis network version when creating a network
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.
Some mostly questions, in general looks very landable.
inv.Register(actors.Version5, vm.ActorsVersionPredicate(actors.Version5), exported5.BuiltinActors()...) | ||
inv.Register(actors.Version6, vm.ActorsVersionPredicate(actors.Version6), exported6.BuiltinActors()...) | ||
inv.Register(actors.Version7, vm.ActorsVersionPredicate(actors.Version7), exported7.BuiltinActors()...) | ||
inv.Register(actors.Version8, vm.ActorsVersionPredicate(actors.Version8), exported8.BuiltinActors()...) |
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.
Do we want v8 go actors in legacy invoker?
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.
We do need this, yes. We ask to register the wrong BuiltinActors() (from v8 go-actors), which we then transform to use the right CIDs and setup the Methods info correctly
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.
In that case, do we maybe want to keep the inline-gen here? (fine without it, just not sure why it got dropped)
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.
Lolol, it got dropped because I thought it was gonna go, and then had to re-add it :P
I'll bring it back.
ret := res.MsgRct.GasUsed | ||
|
||
transitionalMulti := 1.0 | ||
// Overestimate gas around the upgrade |
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.
Can we create an issue to remove (or make more reusable/cleaner/self contained) this after the upgrade epoch?
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.
Yes!
Co-authored-by: Łukasz Magiera <magik6k@users.noreply.github.com>
c235316
to
82e4391
Compare
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.
Let's land this!
94007f8
to
b211c51
Compare
This PR pulls in the changes developed in the feat/nv16 branch into master.