GH-744-Review-3 #563
GH-744-Review-3 #563
Conversation
…are_recorded_in_databases
| #[test] | ||
| #[ignore] |
There was a problem hiding this comment.
What's this? Shouldn't you do it in this card? If you don't know what is causing it we can have a look in the team meeting.
node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/agent_null.rs
Show resolved
Hide resolved
node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/agent_web3.rs
Show resolved
Hide resolved
.../src/blockchain/blockchain_interface/blockchain_interface_web3/lower_level_interface_web3.rs
Outdated
Show resolved
Hide resolved
| @@ -372,7 +376,7 @@ impl BlockchainInterfaceWeb3 { | |||
| ); | |||
|
|
|||
| Ok(RetrievedBlockchainTransactions { | |||
| new_start_block: transaction_max_block_number, | |||
| new_start_block: transaction_max_block_number + 1, | |||
There was a problem hiding this comment.
Yeah this is good. Just take caution for the start_block_fallback_value and its computation. I'm convinced that we've planted a mistake there when somebody decided to add another +1 as well as we do here. Because all together it makes +2. It should take a treatment at the other place by eliminating the plus one, not here.
node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs
Outdated
Show resolved
Hide resolved
| @@ -565,7 +459,7 @@ impl BlockchainBridge { | |||
| if start_block_number == u64::MAX { | |||
| start_block_number | |||
| } else { | |||
| start_block_number + 1u64 // TODO: GH-744 Way are we adding +1 can we just return the same value? | |||
| start_block_number + 1u64 | |||
There was a problem hiding this comment.
Yeah, I believe this is wrong. Careful here. But if you agree with me after I have explained you, do the cleanup.
| @@ -1730,14 +1628,12 @@ mod tests { | |||
| received_payments, | |||
| &ReceivedPayments { | |||
| timestamp: received_payments.timestamp, | |||
| new_start_block: 8675309u64 + 1, | |||
There was a problem hiding this comment.
I believe this is wrong.
It should correspond exactly to the new_start_block in:
RetrievedBlockchainTransactions {
new_start_block: 8675309u64,
transactions: vec![BlockchainTransaction {
block_number: 8675308u64,
from: some_wallet.clone(),
wei_amount: amount,
}],
}
But you had to add one to make the assertion work and it only shows that the internal code is wrong. (We discussed the cause already. You will likely know by now how you should proceed)
No description provided.