feat(f3): manifest retrieval from smart contract #5321
Conversation
hanabi1224
requested review from
LesnyRumcajs and
sudo-shashank
and removed request for
a team
| .parse() | ||
| .expect("Invalid PeerId"), | ||
| f3_contract_address: Some( | ||
| EthAddress::from_str("0x476AC9256b9921C9C6a0fC237B7fE05fe9874F50") |
There was a problem hiding this comment.
The poll interval is managed via an environment variable. Would it make sense to expose the same functionality for the contract address?
| @@ -43,6 +45,7 @@ pub const NEWEST_NETWORK_VERSION: NetworkVersion = NetworkVersion::V17; | |||
| const ENV_FOREST_BLOCK_DELAY_SECS: &str = "FOREST_BLOCK_DELAY_SECS"; | |||
| const ENV_FOREST_PROPAGATION_DELAY_SECS: &str = "FOREST_PROPAGATION_DELAY_SECS"; | |||
| const ENV_PLEDGE_RULE_RAMP: &str = "FOREST_PLEDGE_RULE_RAMP"; | |||
| const DEFAULT_F3_CONTRACT_POLL_INTERVAL: Duration = Duration::from_secs(15 * 60); | |||
There was a problem hiding this comment.
Is it the same as in Lotus?
There was a problem hiding this comment.
f3-sidecar/manifest.go
Outdated
There was a problem hiding this comment.
I'm not sure what are the best practices in Go, but would it make sense to check all pointer parameters in methods that they are not nil? There was recently a nil derefeence in go-f3.
There was a problem hiding this comment.
In our case, the parameters are passed from Rust code which are not nil, the signature types that are generated by rust2go are pointers to avoid lifetime issues
There was a problem hiding this comment.
It might be helpful to comment on this somewhere, given it's not obvious (at least for me).
| pub fn create_eth_call_message(contract: EthAddress) -> EthCallMessage { | ||
| // method ID of activationInformation() | ||
| static METHOD_ID: Lazy<EthBytes> = | ||
| Lazy::new(|| EthBytes::from_str("0x2587660d").expect("Infallible")); |
There was a problem hiding this comment.
We already have this elsewhere. It'd be great to make it a constant and document it, e.g., with a link to the FRC.
There was a problem hiding this comment.
Could you point to me where a similar method id lives?
BTW, added a link to where the constant is defined
There was a problem hiding this comment.
Ah okay, I thought you're duplicating the shed one.
There was a problem hiding this comment.
It'd be great to add the original solidity contract as well for reference.
There was a problem hiding this comment.
Did not find the solidity contract in the original commit, need to ask around.
There was a problem hiding this comment.
The contract lives here: https://github.com/filecoin-project/f3-activation-contract/blob/master/contracts/F3Parameters.sol
Maybe I can just add a link to it?
Co-authored-by: Hubert <hubert@chainsafe.io>
src/rpc/methods/f3/types.rs
Outdated
| Ok(eth_return) | ||
| } | ||
|
|
||
| #[allow(clippy::indexing_slicing)] |
There was a problem hiding this comment.
Wouldn't it be safer to use get? Especially if the data structure changes, we don't want Forest to crash.
Co-authored-by: Hubert <hubert@chainsafe.io>
Summary of changes
Changes introduced in this pull request:
Manually tested the polling flow on mainnet and it worked as expected. To be tested on testnets once the contract is deployed.
Reference issue to close (if applicable)
Closes #5307
Other information and links
Change checklist