Quoting upgrade nodeside #2495
Quoting upgrade nodeside #2495
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } else { | ||
| calculate_cost_for_records(quoting_metrics.close_records_stored) | ||
| }; | ||
| // NB TODO tell happybeing! |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
| verify_data_payment( | ||
| self, | ||
| tx_hash, | ||
| quote_hash, | ||
| // quoting_metrics, // NB TODO use them here @Mick |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
| reward_addr, | ||
| amount, | ||
| Default::default(), // NB TODO remove amounts @Mick |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
| // takes a long time to run | ||
| #[ignore] | ||
| #[test] | ||
| fn address_distribution_sim() { |
There was a problem hiding this comment.
I'd suggest move the above calculate_cost_for_records function into the test mod, and retain these tests.
This serves as simulating tests of what pricing curve to be.
|
|
||
| // vdash metric (if modified please notify at https://github.com/happybeing/vdash/issues): | ||
| info!("Total payment of {storecost:?} atto tokens accepted for record {pretty_key}"); | ||
| info!("Total payment of {reward_amount:?} atto tokens accepted for record {pretty_key}"); |
There was a problem hiding this comment.
this and couple of following logs/prints could be misleading if switched to say pay median or gradual emission scheme.
So, need to make sure the returned reward_amount is really the amount paid to this node directly, or the cost calculated according to the quoting metrics and pricing curve, or something else.
if self.evm_network().verify_data_payment() cann't return with something like (transaction_total_amount, paid_to_node, calculated_from_metrics), then we shall be careful on the wording here.
| } | ||
| all_costs.push((peer_address, payment_address, PaymentQuote::zero())); | ||
| info!("Address {record_address:?} was already paid for according to {peer_address:?}, ending quote request"); | ||
| return Ok(vec![]); |
There was a problem hiding this comment.
this actually creates a hole that if any malicious node replies with RecordExists , it will blocks the upload.
I'd suggest leave it and continue with collect of other responses.
Then decided whether to pay or not by: if find majority of RecordExists responses, or verified can fetch from network
|
Continued in #2497 |
No description provided.