GH-600: Add get_block_number #193

masqrauder · 2022-11-28T06:07:04Z

Introduce the ability to get the current block number of a chain.
Utilize batch transport to request block number and transaction logs within the same blockchain service provider API batch request.
Incrementally scan the blockchain for payments and use the upper block number bound as required.
Handle block number range errors reported by blockchain service providers to allow recovery and catch up to the current block if the node falls behind.

bertllll · 2022-12-07T11:09:13Z

node/src/blockchain/blockchain_interface.rs

@@ -254,12 +276,22 @@ where
            )
            .build();

-        let log_request = self.web3.eth().logs(filter);
+        let end_block_number = match Option::from(end_block) {


You're wrapping the value so that you take the covering away just a moment later. Instead, you can ignore the Option and just write:

let end_block_number = match end_block { BlockNumber::Number(eb) => eb.as_u64(), _ => start_block + 1, };

Also, related to the suggestion at args, this name should probably change a bit too, so that it better fits to the choice for the name of the argument up there.

What name do you recommend instead of end_block_number? It's really a fallback value that is used in the event there's an error in retrieving the latest block number from the chain. This value may also be changed after GH-585 where the blockchain service might provide a maximum block count delta from the start block as a limitation we have to use on subsequent calls.

Actually fallback_end_block_number or somehow shorter looks good to me. (I know you've got better sense for the language, so I'm not suggesting nothing but that the word fallback would be nice to have in there). What do you think? It might stop making people wonder about what that is.

bertllll · 2022-12-07T19:28:42Z

node/src/blockchain/blockchain_interface.rs

@@ -232,6 +253,7 @@ where
    fn retrieve_transactions(
        &self,
        start_block: u64,
+        end_block: BlockNumber,


I do recommend to use a clearer name here, because variables like this one are mentioned a few times through this function. It's not so easy to read and understand.

My suggestions: minimal_end_block, null_end_block, smallest_possible_end_block...I'm quite sure you will be better at finding an appropriate name.

I could potentially wrap start_block and end_block in a new block range datatype if you think it warrants it. The web3 interface uses the BlockNumber enum for from_block and to_block. For our use case, if we were to use BlockNumber for start_block some of the enum values don't make sense like BlockNumber::Pending We could interpret BlockNumber::Earliest as the MASQ smart contract block number, but that would not fit the true web3 meaning. But then again for the most part we want to use BlockNumber::Latest for someone starting node for the first time with a new wallet address.

bertllll · 2022-12-07T19:36:11Z

node/src/blockchain/blockchain_interface.rs

-                future::result::<RetrievedBlockchainTransactions, BlockchainError>(match logs {
+        match self.web3_batch.transport().submit_batch().wait() {
+            Ok(_) => {
+                let block_number = match block_request.wait() {


Maybe a name like: possibly_updated_block_number...

Yes, this working value name could be better as response_block_number because it's in response to our block number request call. This value may be impacted by GH-585 error handling for error responses providing a max block range value.

Yeah, in general, I'm all for naming variables with more meaningful names, if they are somehow specific or different from other ones. So I think it'd be an improvement.

bertllll · 2022-12-07T22:46:42Z

node/src/blockchain/blockchain_interface.rs

+            Ok(latest_block_number) => assert_eq!(latest_block_number, U64::from(31_682_662u64)),
+            Err(e) => assert_eq!(
+                e,
+                BlockchainError::QueryFailed("unexpected failure".to_string())


This is a strange way of handling an unfavorable result.
Instead of an assertion which tends to be used for an expected behavior, it's more appropriate to use panic!(), printing the error, whatever it is, with a simple msg.

I' making the point because I'm assuming this test focuses on a success on this method call...

You're right. I should think more in terms of production vs test code.

bertllll · 2022-12-07T22:57:11Z

node/src/blockchain/blockchain_interface.rs

+        let actual_arguments: Vec<String> = actual_arguments
+            .into_iter()
+            .map(|arg| serde_json::to_string(&arg).unwrap())
+            .collect();


Given the params of the prepare method are hard-coded, that is an empty vector, and so you will always get the same result for such a param assertion I'd welcome a simpler way of going through it.

You can skip the step above, trying to iterate over an empty vector, leaving:

let (method_name, actual_arguments) = prepare_params.remove(0); assert_eq!(method_name, "eth_blockNumber".to_string()); let expected_arguments: Vec<Value> = vec![]; assert_eq!(actual_arguments, expected_arguments);

The same should be preferably done with all the following tests too.

ok. The point of this part of the test was to confirm that there are no arguments passed to the method. I was keeping the tests consistent so that you don't have to mentally switch gears back and forth. I guess this had the opposite effect. If there's a better way to ensure the test breaks appropriately if the arguments change, then I'd like to follow that. I was too focused on trying to get data type held by the vector to be consistent I didn't think of what you have.

You know, for now the assertion testing an empty vector is good enough. If it changes it will definitely breaks. But again, that parameter is so-far constant. One could point out it could change with a different version of the library.

For clarity, I didn't complain about the fact you want to assert on this parameter I just didn't see a point in converting the potential values to a different, maybe better assertable data types. That step just prolongs the test even though it makes no difference. The vector is empty at any case.

node/src/blockchain/blockchain_interface.rs

bertllll · 2022-12-08T00:04:29Z

node/src/blockchain/blockchain_bridge.rs

+        let end_block = match self.blockchain_interface.get_block_number() {
+            Ok(eb) => BlockNumber::Number(eb),
+            Err(_) => BlockNumber::Latest,
+        };


I'm reluctant to believe that both of those branches of the match are adequately tested. I miss one test.

node/src/blockchain/blockchain_interface.rs

dnwiebe

Resume at blockchain_interface:287 green

node/src/blockchain/blockchain_interface.rs

@@ -232,6 +253,7 @@ where
    fn retrieve_transactions(
        &self,
        start_block: u64,
+        end_block: BlockNumber,


dnwiebe · 2022-12-14T13:25:21Z

node/src/blockchain/blockchain_interface.rs

@@ -275,40 +307,51 @@ where
                            let transactions: Vec<BlockchainTransaction> = logs


This hunk of code seems like something that could live in an outboard method and make the caller clearer.

dnwiebe · 2022-12-14T13:26:26Z

node/src/blockchain/blockchain_interface.rs

-                                });
+                            // transactions, in which case use end_block, unless get_latest_block()
+                            // was not successful.
+                            let transaction_max_block_number = if transactions.is_empty() {


This hunk of code seems like something that could live in an outboard method and make the caller clearer.

node/src/blockchain/blockchain_interface.rs

bertllll

Continue at blockchain_interface.rs: blockchain_interface_non_clandestine_can_transfer_tokens

bertllll · 2022-12-23T10:13:07Z

node/src/blockchain/blockchain_interface.rs

+            .filter_map(|log: &Log| match log.block_number {
+                None => None,
+                Some(block_number) => {
+                    to_gwei(U256::from(log.data.0.as_slice())).map(|gwei_amount| {


Maybe this idea is silly but what if you put the value log.data.0 on a different line and give it a name through using a variable. Readability of the code might get better. I'll let you decide if it's worth it. But you know, only the person who will go to the source code of the library will really know what this is exactly.

bertllll · 2022-12-23T10:22:33Z

node/src/blockchain/blockchain_interface.rs

+                        BlockchainTransaction {
+                            block_number: block_number.as_u64(),
+                            from: Wallet::from(log.topics[1]),
+                            gwei_amount,


One important thing here, by the time of this writing we've got already "in-wei counting" system implemented. That means that the other Nodes will probably pay you something different than just clear gwei values. These numbers moved between Nodes are going to be much more accurate. So you don't want to lose the accuracy here by conversion to gwei. You'll have to change the field's name and the data type to u128, plus get rid of the conversion.

It might be the case that it will be automatically taken care of by doing the merge (I might have adjusted this code already...well, given your method is now concise and placed alone...the merge might not affect this code...but its mostly the same so maybe yes.). Please check it out anyway.

bertllll · 2022-12-23T10:24:38Z

node/src/blockchain/blockchain_interface.rs

@@ -640,13 +716,13 @@ mod tests {
            std::env::set_var("SIMPLESERVER_THREADS", "1");
            let (tx, rx) = unbounded();
            let _ = thread::spawn(move || {
-                let bodies_arc = Arc::new(Mutex::new(bodies));
+                let bodies_arc: Arc<Mutex<Vec<Vec<u8>>>> = Arc::new(Mutex::new(bodies));


This type definition shouldn't be necessary but if you want it to stay, okay.

bertllll · 2022-12-23T10:26:53Z

node/src/blockchain/blockchain_interface.rs

                Server::new(move |req, mut rsp| {
                    if req.headers().get("X-Quit").is_some() {
                        panic!("Server stop requested");
                    }
                    tx.send(req).unwrap();
-                    let body = bodies_arc.lock().unwrap().remove(0);
+                    let body: Vec<u8> = bodies_arc.lock().unwrap().remove(0);


Similarly...
We usually don't do this if the compiler knows the truth and you as a human can read it out of the combination of the function signature and the code in the body. I'm saying that just for your better knowledge. I don't find it serious though and won't request a change.

bertllll · 2022-12-23T10:30:24Z

node/src/blockchain/blockchain_interface.rs

-            format!("\"0x000000000000000000000000{}\"", &to[2..]),
-            bodies[0]["params"][0]["topics"][2].to_string(),
+            vec![format!("[{{\"id\":0,\"jsonrpc\":\"2.0\",\"method\":\"eth_blockNumber\",\"params\":[]}},{{\"id\":1,\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{{\"address\":\"0x384dec25e03f94931767ce4c3556168468ba24c3\",\"fromBlock\":\"0x2a\",\"toBlock\":\"0x400\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x000000000000000000000000{}\"]}}]}}]", &to[2..])],
+            bodies //bodies[0]["params"][0]["topics"][1].to_string(),


We have a policy of putting the value being asserted on the left side of the assert_eq!() macro.

So you're saying actual then expected? I think these escaped the initial policy switch. I think this code was originally written with expected then actual order.

I see. Yeah, sometimes we still come across some old tests where it's reversed.

Confirming that this is the preferred style: assert_eq!(actual, expected)

bertllll · 2022-12-23T10:41:05Z

node/src/blockchain/blockchain_interface.rs

-            format!("\"0x000000000000000000000000{}\"", &to[2..]),
-            bodies[0]["params"][0]["topics"][2].to_string(),
+            vec![format!("[{{\"id\":0,\"jsonrpc\":\"2.0\",\"method\":\"eth_blockNumber\",\"params\":[]}},{{\"id\":1,\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{{\"address\":\"0x384dec25e03f94931767ce4c3556168468ba24c3\",\"fromBlock\":\"0x2a\",\"toBlock\":\"0x400\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x000000000000000000000000{}\"]}}]}}]", &to[2..])],
+            bodies //bodies[0]["params"][0]["topics"][1].to_string(),


Also this whole structure:
vec![format!("[{{\"id\":0,\"jsonrpc\":\"2.0\",\"method\":\"eth_blockNumber\",\"params\":[]}},{{\"id\":1,\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{{\"address\":\"0x384dec25e03f94931767ce4c3556168468ba24c3\",\"fromBlock\":\"0x2a\",\"toBlock\":\"0x400\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x000000000000000000000000{}\"]}}]}}]", &to[2..])],
which you use in the assertion as the template is equal to the variable bodies.

That's why the comment //bodies[0]["params"][0]["topics"][1].to_string(), is pointless what more, it's confusing. I'm asking you to remove it.

bertllll · 2022-12-23T10:41:52Z

node/src/blockchain/blockchain_interface.rs

-            format!("\"0x000000000000000000000000{}\"", &to[2..]),
-            bodies[0]["params"][0]["topics"][2].to_string(),
+            vec![format!("[{{\"id\":0,\"jsonrpc\":\"2.0\",\"method\":\"eth_blockNumber\",\"params\":[]}},{{\"id\":1,\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{{\"address\":\"0x384dec25e03f94931767ce4c3556168468ba24c3\",\"fromBlock\":\"0x2a\",\"toBlock\":\"0x400\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x000000000000000000000000{}\"]}}]}}]", &to[2..])],
+            bodies,


Please also reverse the asserted value with the asserting one.

bertllll · 2022-12-23T10:44:02Z

node/src/blockchain/blockchain_interface.rs

        );
        assert_eq!(
            result,
            RetrievedBlockchainTransactions {
-                new_start_block: 0x4be663 + 1,
+                new_start_block: 0x4be663,


Is this block going to be included in the next scan? Because if yes then it is the same as the block of the last transaction we found and so we would pick that transaction again. Please verify.

bertllll · 2022-12-23T10:45:34Z

node/src/blockchain/blockchain_interface.rs

        let (event_loop_handle, transport) = Http::with_max_parallel(
-            &format!("http://{}:{}", &Ipv4Addr::LOCALHOST, port),
+            &format!("http://{}:{}", &Ipv4Addr::LOCALHOST.to_string(), port),


.to_string() inside the format!() macro is redundant. Format will do it itself.

bertllll · 2022-12-28T17:36:46Z

node/src/blockchain/blockchain_interface.rs

+        assert_eq!(
+            actual_arguments,
+            vec![String::from(
+                r#""0xf8a901851bf08eb00082dbe894384dec25e03f94931767ce4c3556168468ba24c380b844a9059cbb00000000000000000000000000000000000000000000000000626c61683132330000000000000000000000000000000000000000000000000000082f79cd900029a0d4ecb2865f6a0370689be2e956cc272f7718cb360160f5a51756264ba1cc23fca005a3920e27680135e032bb23f4026a2e91c680866047cf9bbadee23ab8ab5ca2""#


Not sure what the seemingly blank line (or really white spaces?) at the beginning of the line 1301 means. I'd feel better if you had a look at it to check out for some strange formatting and to confirm it's false alarm 🙂

I don't know. It's just the formatter adding the indentation.

bertllll · 2022-12-28T17:42:15Z

node/src/blockchain/blockchain_interface.rs

+        assert!(prepare_params.is_empty());
+        let actual_arguments: Vec<String> = actual_arguments
+            .into_iter()
+            .map(|arg| serde_json::to_string(&arg).unwrap())


You do this as often that it would have some positive effect if you created a helper function (used only this test module) which would take the a parameter with the type of actual_arguments and it would return a Vec<String>.

It will take care of duplication and make the tests slightly shorter.

bertllll · 2022-12-28T17:44:52Z

node/src/blockchain/blockchain_interface.rs

+
+    #[test]
+    fn non_clandestine_can_fetch_latest_block_number_successfully() {
+        let prepare_params_arc: Arc<Mutex<Vec<(String, Vec<Value>)>>> =


The explicit type should not be necessary to understand the test and the compiler will not complain too.

node/src/blockchain/blockchain_interface.rs

bertllll · 2022-12-28T18:04:58Z

node/src/blockchain/blockchain_interface.rs

+        );
+
+        match subject.get_block_number() {
+            Ok(latest_block_number) => assert_eq!(latest_block_number, U64::from(31_682_662u64)),


Could you please interpret this expected number also as a hexadecimal number? The understanding is gonna be easier thanks to the fast comparison of those two values, even though as different data types.

U64::from(0x1e37066u64)

+1

Also, the test would be easier to read if you untangled the Assert from the Act.

node/src/blockchain/blockchain_interface.rs

bertllll · 2022-12-28T18:15:11Z

node/src/blockchain/test_utils.rs

+    send_batch_params: Arc<Mutex<Vec<Vec<(RequestId, rpc::Call)>>>>,
+    send_batch_results: RefCell<Vec<Vec<Result<rpc::Value, web3::Error>>>>,
+    //to check if we hold a true descendant of a certain initial instance
+    reference_counter_opt: Option<Arc<()>>,


You probably don't need the two last lines. I implemented them but your version of the codebase doesn't need it.

It would stay idle for a while and it's better not to add it in now.

node/src/blockchain/test_utils.rs

dnwiebe

Resume at blockchain_interface:822 green.

dnwiebe · 2023-02-07T12:39:27Z

node/src/blockchain/blockchain_bridge.rs

+            Err(e) => {
+                warning!(
+                    self.logger,
+                    "Using 'latest' block number instead of a literal number. {:?}",


Is it going to be clear from context what the latest block number is being used for, and what the consequences of that will be?

Maybe it should be informational instead of a warning? It's not meant to sound bad or dangerous; only that we couldn't fetch the current block number and so now we're just going to query using BlockNumber::Latest.

Yeah, it sounds like something that ought to be an info!() instead of a warning!().

node/src/blockchain/blockchain_bridge.rs

node/src/blockchain/test_utils.rs

node/src/blockchain/blockchain_interface.rs

dnwiebe · 2023-02-07T13:26:43Z

node/src/blockchain/blockchain_interface.rs

        assert_eq!(
-            format!("\"0x000000000000000000000000{}\"", &to[2..]),
-            bodies[0]["params"][0]["topics"][2].to_string(),
+            bodies,


Is there any danger that this assert will be disrupted by JSON formatting? Say, if somehow we get spaces after the commas, or newlines, or indentation, or whatever? If so, it might be better to compare these two as parsed objects rather than JSON strings.

From Bert: you could eliminate a lot of the backslashes if you used r#" quotes.

If this breaks because of json formatting I suspect that means the block chain service has changed and we'd want the broken test to warn us of that. The raw jsonrpc response is how we see the data and is very compact without extra spaces and new lines. Do you have another suggestion to do instead?

I saw Bert's comment regarding the format!() I could remove the format to eliminate the escaping but this means the &to[2..] address would have to be hardcoded in an additional place. Do you prefer it hardcoded?

I wouldn't give the life for being right on this but I think it would allow you to use format!() also with r#"...text..."...text..."# literals that contain double quotes and it even can have a placeholder in it like {} or {:?} and it should still work as long as you supply that many arguments as how many placeholders you put in there. You can try but maybe not worth it.

dnwiebe · 2023-02-08T12:33:37Z

node/src/blockchain/blockchain_interface.rs

    }

    #[test]
-    fn blockchain_interface_non_clandestine_retrieve_transactions_returns_an_error_if_a_response_with_data_that_is_too_long_is_returned(
+    fn blockchain_interface_non_clandestine_retrieve_transactions_ignores_transaction_logs_that_have_no_block_number(


I understand that transaction logs without block numbers are probably not a pressing real-world problem, and that you have to handle them only because we're not allowed to crash because of something that comes in from outside; but just in case it does happen, would it be worth writing an error log?

node/src/blockchain/blockchain_interface.rs

dnwiebe · 2023-02-08T12:50:43Z

node/src/blockchain/blockchain_interface.rs

+        );
+
+        match subject.get_block_number() {
+            Ok(latest_block_number) => assert_eq!(latest_block_number, U64::from(31_682_662u64)),


+1

Also, the test would be easier to read if you untangled the Assert from the Act.

dnwiebe

I noticed a few things, but nothing that would cause real trouble, so I'll approve. It'd be nice if you'd take a look through the comments anyway, though.

dnwiebe · 2023-03-08T12:22:52Z

node/src/blockchain/blockchain_interface.rs

                        if logs
                            .iter()
                            .any(|log| log.topics.len() < 2 || log.data.0.len() > 32)
                        {
                            warning!(
                                logger,
                                "Invalid response from blockchain server: {:?}",
-                                logs
+                                logs.clone()


Bert wonders: does the macro really take ownership here?

It doesn't. I removed the .clone()

dnwiebe · 2023-03-08T12:25:10Z

node/src/blockchain/blockchain_interface.rs

-                        transactions: vec![],
-                    }),
+                    Err(e) => {
+                        error!(self.logger, "Retrieving transactions: {}", e.to_string());


I think the .to_string() is unnecessary here. Doesn't the {} placeholder automatically call the Display implementation, which is what .to_string() does?

(Bert says, "Hell, yes!")

node/src/blockchain/blockchain_interface.rs

dnwiebe · 2023-03-08T12:38:56Z

node/src/blockchain/blockchain_interface.rs

            .collect();
        assert_eq!(
-            bodies,
-            vec![format!("[{{\"id\":0,\"jsonrpc\":\"2.0\",\"method\":\"eth_blockNumber\",\"params\":[]}},{{\"id\":1,\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{{\"address\":\"0x384dec25e03f94931767ce4c3556168468ba24c3\",\"fromBlock\":\"0x2a\",\"toBlock\":\"0x400\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x000000000000000000000000{}\"]}}]}}]", &to[2..])],
+            bodies[0],


I liked the strength of the assertion that bodies was equal to vec[-just one thing-], because it included the requirement that there could only be one element in the vector.

Also, you could DRY that back up, if you wanted, by doing something like

let expected_body_prefix = -complicated #r string ending in zeros-;
let expected_body = format!("{}{}", expected_body_prefix, &to[2..]);
assert_eq!(bodies, vec[expected_body]);

I used your recommended changes but could not do so without introducing an expected_body_suffix for the closing JSON syntax. It would be nice if format!() could work with raw Strings.

node/src/blockchain/blockchain_interface.rs

dnwiebe · 2023-03-08T12:53:45Z

node/src/blockchain/blockchain_interface.rs

@@ -1011,6 +1010,10 @@ mod tests {
                transactions: vec![]
            })
        );
+        let test_log_handler = TestLogHandler::new();
+        test_log_handler.assert_logs_contain_in_order(vec![


This looks as though it would work fine, but why .assert_logs_contain_in_order instead of .exists_log_containing?

dnwiebe · 2023-03-08T12:56:36Z

node/src/blockchain/blockchain_interface.rs

@@ -2223,19 +2236,21 @@ mod tests {
            TEST_DEFAULT_CHAIN,
        );

-        match subject.get_block_number() {
+        let expected_error = match subject.get_block_number() {


I think you can do subject.get_block_number().unwrap_err() here and ditch the match, if you want.

bertllll

I'd like to see a change making the assertion simpler and obvious at what is important in the error message. Also without if statements involved that aren't becoming to assertions in general.

Hopefully you will agree 🙏

node/src/blockchain/blockchain_interface.rs

bertllll · 2023-04-13T14:58:39Z

node/src/blockchain/blockchain_bridge.rs

-            .retrieve_transactions(start_block, &msg.recipient);
+        let retrieved_transactions =
+            self.blockchain_interface
+                .retrieve_transactions(start_block, end_block, &msg.recipient);
        match retrieved_transactions {
            Ok(transactions) => {
                if let Err(e) = self


Please put a debug log here announcing the newly picked start block before you write it into the db. It's going to make debugging a lot easier for the future.

bertllll · 2023-04-13T15:06:24Z

node/src/blockchain/blockchain_interface.rs

+        match self.web3_batch.transport().submit_batch().wait() {
+            Ok(_) => {
+                let response_block_number = match block_request.wait() {
+                    Ok(block_nbr) => block_nbr.as_u64(),


Also here, this would be such good information for a debugging person. Please add the debug log saying what current head block was reported from the blockchain.

GH-600: Add get_block_number GH-600: Address PR feedback GH-585: Experiment with adapting test for platform specific error codes GH-585: Provide Windows platform specific error code and message GH-600: Rebase master into branch

* Starts Accountant * Adds Receivables scan timeout (may become a configuration parameter in another card) * Stores `max_block_count` in the config table when processing error messages providing a limit * Adds Max Block Count to configuration response * Increments CURRENT_SCHEMA_VERSION * Adds schema migration for `max_block_count` * Improves blockchain scan error handling and logging

kauri-hero

adding approval for PR merge

masqrauder requested review from bertllll, FinsaasGH, kauri-hero, utkarshg6 and dnwiebe November 28, 2022 06:07

masqrauder force-pushed the GH-600 branch from 4f38144 to 3421fb4 Compare November 28, 2022 06:19

bertllll reviewed Dec 8, 2022

View reviewed changes

dnwiebe reviewed Dec 12, 2022

View reviewed changes

node/src/blockchain/blockchain_interface.rs Show resolved Hide resolved

node/src/blockchain/blockchain_interface.rs

@@ -232,6 +253,7 @@ where

fn retrieve_transactions(

&self,

start_block: u64,

end_block: BlockNumber,

This comment was marked as resolved.

Sign in to view

dnwiebe reviewed Dec 14, 2022

View reviewed changes

bertllll reviewed Dec 23, 2022

View reviewed changes

bertllll reviewed Dec 28, 2022

View reviewed changes

masqrauder force-pushed the GH-600 branch from a51d1b5 to a4c0b43 Compare February 6, 2023 04:55

dnwiebe reviewed Feb 7, 2023

View reviewed changes

dnwiebe reviewed Feb 8, 2023

View reviewed changes

dnwiebe approved these changes Mar 8, 2023

View reviewed changes

bertllll approved these changes Apr 6, 2023

View reviewed changes

node/src/blockchain/blockchain_interface.rs Outdated Show resolved Hide resolved

bertllll approved these changes Apr 6, 2023

View reviewed changes

bertllll reviewed Apr 13, 2023

View reviewed changes

masqrauder force-pushed the GH-600 branch 3 times, most recently from 8a01f3a to 2c93c9b Compare May 8, 2023 02:48

masqrauder force-pushed the GH-600 branch from 2c93c9b to 3de447a Compare May 8, 2023 06:00

masqrauder force-pushed the GH-600 branch from 3de447a to 86e4f9c Compare June 4, 2023 17:44

masqrauder force-pushed the GH-600 branch from 86e4f9c to cb0075b Compare July 3, 2023 04:33

masqrauder force-pushed the GH-600 branch from cb0075b to 530c97c Compare July 23, 2023 19:30

masqrauder force-pushed the GH-600 branch 2 times, most recently from 027efbc to 849d8be Compare September 6, 2023 06:36

masqrauder force-pushed the GH-600 branch 2 times, most recently from 0a6f901 to 673763c Compare September 23, 2023 19:02

Squashing:

6530e4d

GH-600: Add get_block_number GH-600: Address PR feedback GH-585: Experiment with adapting test for platform specific error codes GH-585: Provide Windows platform specific error code and message GH-600: Rebase master into branch

masqrauder added 9 commits October 4, 2023 03:00

GH-600: Handle expected batch, got single response

4876be3

Resolve rebase issues

1e20aa6

GH-600: Adapt to new Pokt message changes

094f0b3

Use start_block in the event of an error

1fa8c4d

GH-600: Return blockchain scan error to update max_block_count

66eff5a

GH-600: Add test for scan time out

b4a4fca

GH-600: Revert scanner changes

71bc0ce

GH-600: Address review feedback

d101ff5

masqrauder force-pushed the GH-600 branch from 673763c to d101ff5 Compare October 4, 2023 07:00

masqrauder added 2 commits October 7, 2023 16:50

GH-600: Address review 6 feedback

9cbf321

GH-600: Use From traits to be concise

dc061d7

kauri-hero requested review from kauri-hero and removed request for utkarshg6, kauri-hero and FinsaasGH October 9, 2023 23:01

kauri-hero approved these changes Oct 10, 2023

View reviewed changes

masqrauder merged commit 44193ef into master Oct 10, 2023
6 checks passed

masqrauder deleted the GH-600 branch October 15, 2023 21:20

		@@ -275,40 +307,51 @@ where
		let transactions: Vec<BlockchainTransaction> = logs

GH-600: Add get_block_number #193

GH-600: Add get_block_number #193

Conversation

masqrauder commented Nov 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masqrauder Dec 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnwiebe left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bertllll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnwiebe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnwiebe Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masqrauder Feb 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnwiebe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bertllll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kauri-hero left a comment

Choose a reason for hiding this comment

masqrauder Dec 9, 2022 •

edited

Loading

dnwiebe Mar 7, 2023 •

edited

Loading

masqrauder Feb 12, 2023 •

edited

Loading