Evm integration step 2: working Node #2144
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
evmlib/src/wallet.rs
Outdated
| Ok(EthereumWallet::from(signer)) | ||
| } | ||
|
|
||
| // TODO(optimization): Find a way to reuse/persist contracts and/or a provider without the wallet nonce going out of sync |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
| // let new_balance = AttoTokens::from_atto(wallet_original_balance - total_cost.as_atto()); | ||
| // info!("Verifying new balance on paying wallet is {new_balance} ..."); | ||
| // let paying_wallet = wallet_client.into_wallet(); | ||
| // // assert_eq!(paying_wallet.balance(), new_balance);// TODO adapt to evm |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note test
| // ); | ||
| // println!("Verifying new balance on paying wallet is now {new_balance} ..."); | ||
| // let paying_wallet = wallet_client.into_wallet(); | ||
| // // TODO adapt to evm |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note test
| // ); | ||
| // } | ||
|
|
||
| // // TODO adapt to evm |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note test
| retrieved_reg.ops.len() | ||
| ); | ||
| println!("current local cached ops length is {}", register.ops.len()); | ||
| // // TODO adapt to evm |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note test
grumbach
force-pushed
the
evm_integration_step_2
branch
from
4512988 to
347f0f8
Compare
| @@ -258,12 +288,12 @@ fn main() -> Result<()> { | |||
| let restart_options = rt.block_on(async move { | |||
| let mut node_builder = NodeBuilder::new( | |||
| keypair, | |||
| rewards_address, | |||
There was a problem hiding this comment.
are we still carry out forwarding ?
if so, the previous usage of set owner as None was requested from community to leave the optioin that node owner could keep their earnings for other usage.
There was a problem hiding this comment.
We dont have forwarding anymore
| node_socket_addr, | ||
| bootstrap_peers, | ||
| opt.local, | ||
| root_dir, | ||
| opt.owner.clone(), |
There was a problem hiding this comment.
if no longer used, remove the member field.
| .get_node_wallet_balance() | ||
| .expect("Failed to get node wallet balance") | ||
| .as_nano(), | ||
| wallet_balance: 0, // NB TODO: Implement this using metrics data? |
There was a problem hiding this comment.
put as a remind, as this seems shall be resolved before merge
There was a problem hiding this comment.
Maybe should remove it as there is no more wallet in the node? I've added a new event to report rewards so we can still have a view into this.
| debug!("Received payment of {received_fee:?} for {pretty_key}"); | ||
| debug!("Payment quote signature is valid for record {pretty_key}"); | ||
|
|
||
| // verify quote timestamp |
There was a problem hiding this comment.
the following block of code can actually be within a function of quote, say quote.is_expired()
There was a problem hiding this comment.
We don't verify if the quote is expired here, the smart contract does it
| // check if payment is valid on chain | ||
| debug!("Verifying payment for record {pretty_key}"); | ||
| self.evm_network() | ||
| .verify_chunk_payment( |
There was a problem hiding this comment.
does it also cover the verification of the royalty_fee ?
There was a problem hiding this comment.
There are no royalties anymore
|
|
||
| Ok(()) | ||
| } | ||
| // // Copyright 2024 MaidSafe.net limited. |
There was a problem hiding this comment.
the fund transfer shall still be supported ?
or, it's now totally using third party wallet to do this ?
There was a problem hiding this comment.
For now it will be left to third party wallets, until we decide to re-introduce it.
| use xor_name::XorName; | ||
|
|
||
| #[tokio::test] | ||
| async fn storage_payment_succeeds() -> Result<()> { |
There was a problem hiding this comment.
the scenarioes here are valid client usage, which shall be covered against a local network during CI.
they need to be updated to using the new EVM.
However, wondering if there will be gas fee incurred ?
| @@ -870,6 +874,12 @@ pub enum LocalSubCmd { | |||
| /// services, which in this case would be 5. The range must also go from lower to higher. | |||
| #[clap(long, value_parser = PortRange::parse)] | |||
| rpc_port: Option<PortRange>, | |||
| /// Specify the wallet address that will receive the node's earnings. | |||
| #[clap(long)] | |||
| rewards_address: RewardsAddress, | |||
There was a problem hiding this comment.
seems all owner usage is actually no longer useful.
shall they be removed to avoid misuse/confusing ?
There was a problem hiding this comment.
Do you mean owner_prefix?
grumbach
changed the title
DO NOT MERGE YET!
PLEASE MERGE THIS AFTER #2142
The objective being to have a fully working network with evm payments. Next step is to rewrite the cli and fix/trim remaining wallet and native currency traces.