-
Notifications
You must be signed in to change notification settings - Fork 157
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
chore(test): bump lotus version in CI tests #4286
Conversation
|
||
use crate::rpc::types::SectorOnChainInfo; | ||
|
||
pub trait MinerStateExt { |
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 feel it makes things much easier to move fil_actors_inteface
code into forest
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.
Doesn't it make it inconsistent? I don't have a strong opinion on where this should be, but I'd rather have all this in one place and not scattered randomly.
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 could have a vote maybe. I prefer the interface in forest to
- avoid publishing
fil_actor_*
crates on every change. - avoid the shim(e.g.
miner::SectorOnChainInfo
) in the currentfil_actors_interface
which does not implementLotusJson
orJsonSchema
and we have to re-define a same one inforest
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.
Feel free to propose an initiative to change this. I only would like this to be in the same place to avoid wildly jumping between repos.
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.
@hanabi1224 I prefer the actor interface to be in forest as well.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] | ||
#[serde(rename_all = "PascalCase")] | ||
pub struct SectorOnChainInfo { |
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 assume there's no way of not re-defining this?
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 so, this is the struct for LotusJson serde.
|
||
use crate::rpc::types::SectorOnChainInfo; | ||
|
||
pub trait MinerStateExt { |
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.
@hanabi1224 I prefer the actor interface to be in forest as well.
@@ -407,9 +407,9 @@ pub struct Block { | |||
} | |||
|
|||
impl Block { | |||
pub fn new(has_transactions: bool) -> Self { | |||
pub fn new(has_transactions: bool, tipset_len: usize) -> Self { |
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.
Summary of changes
Changes introduced in this pull request:
SectorOnChainInfo
structBlacklistEthGetBlockByNumber
gasLimit
inEthGetBlockByNumber
Failures(gasLimit not match in EthGetBlockByNumber):
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist