Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FlowBlockSeal protobuf updates #122

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

FlowBlockSeal protobuf updates #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
218b5e2
FlowBlockSeal protobuf updates
lealobanov Oct 20, 2024
b7308f1
Lint
lealobanov Oct 20, 2024
1f4ab77
Add handling for null pointer exception
lealobanov Oct 20, 2024
575a937
WIP: Get account balance endpoints
lealobanov Oct 21, 2024
aa8a226
WIP: Get account balance endpoints
lealobanov Oct 21, 2024
93e6aee
Update unit tests for account balance endpoints
lealobanov Oct 22, 2024
26f6a89
Update integration tests
lealobanov Oct 22, 2024
90ce567
Setup kotlin examples
lealobanov Oct 22, 2024
397339e
Setup java examples
lealobanov Oct 23, 2024
5e02ad8
Merge pull request #134 from onflow/get-account-balance
franklywatson Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .../java/org/onflow/examples/java/getAccountBalance/GetAccountBalanceAccessAPIConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.onflow.examples.java.getAccountBalance;

import org.onflow.flow.sdk.FlowAccessApi;
import org.onflow.flow.sdk.FlowAddress;

public class GetAccountBalanceAccessAPIConnector {
private final FlowAccessApi accessAPI;

public GetAccountBalanceAccessAPIConnector(FlowAccessApi accessAPI) {
this.accessAPI = accessAPI;
}

public long getBalanceAtLatestBlock(FlowAddress address) {
FlowAccessApi.AccessApiCallResponse<Long> response = accessAPI.getAccountBalanceAtLatestBlock(address);

if (response instanceof FlowAccessApi.AccessApiCallResponse.Success) {
return ((FlowAccessApi.AccessApiCallResponse.Success<Long>) response).getData();
} else {
FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) response;
throw new RuntimeException(errorResponse.getMessage(), errorResponse.getThrowable());
}
}
Comment on lines +13 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling and add documentation.

Consider these improvements:

  1. Create a custom exception instead of using generic RuntimeException
  2. Add parameter validation
  3. Add method documentation

Apply these changes:

+/**
+ * Retrieves the account balance at the latest block.
+ *
+ * @param address The Flow address to query the balance for
+ * @return The account balance
+ * @throws AccountBalanceException if the balance retrieval fails
+ * @throws IllegalArgumentException if address is null
+ */
 public long getBalanceAtLatestBlock(FlowAddress address) {
+    if (address == null) {
+        throw new IllegalArgumentException("address cannot be null");
+    }
+
     FlowAccessApi.AccessApiCallResponse<Long> response = accessAPI.getAccountBalanceAtLatestBlock(address);
 
     if (response instanceof FlowAccessApi.AccessApiCallResponse.Success) {
         return ((FlowAccessApi.AccessApiCallResponse.Success<Long>) response).getData();
     } else {
         FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) response;
-        throw new RuntimeException(errorResponse.getMessage(), errorResponse.getThrowable());
+        throw new AccountBalanceException("Failed to retrieve balance at latest block", errorResponse.getMessage(), errorResponse.getThrowable());
     }
 }

Create a custom exception class:

public class AccountBalanceException extends RuntimeException {
    private final String errorDetails;

    public AccountBalanceException(String message, String errorDetails, Throwable cause) {
        super(message, cause);
        this.errorDetails = errorDetails;
    }

    public String getErrorDetails() {
        return errorDetails;
    }
}


public long getBalanceAtBlockHeight(FlowAddress address, long height) {
FlowAccessApi.AccessApiCallResponse<Long> response = accessAPI.getAccountBalanceAtBlockHeight(address, height);

if (response instanceof FlowAccessApi.AccessApiCallResponse.Success) {
return ((FlowAccessApi.AccessApiCallResponse.Success<Long>) response).getData();
} else {
FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) response;
throw new RuntimeException(errorResponse.getMessage(), errorResponse.getThrowable());
}
}
Comment on lines +24 to +33
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add parameter validation and improve error handling.

Similar to getBalanceAtLatestBlock, this method needs documentation and better error handling. Additionally, validate the block height parameter.

Apply these changes:

+/**
+ * Retrieves the account balance at a specific block height.
+ *
+ * @param address The Flow address to query the balance for
+ * @param height The block height to query at
+ * @return The account balance
+ * @throws AccountBalanceException if the balance retrieval fails
+ * @throws IllegalArgumentException if address is null or height is negative
+ */
 public long getBalanceAtBlockHeight(FlowAddress address, long height) {
+    if (address == null) {
+        throw new IllegalArgumentException("address cannot be null");
+    }
+    if (height < 0) {
+        throw new IllegalArgumentException("block height cannot be negative");
+    }
+
     FlowAccessApi.AccessApiCallResponse<Long> response = accessAPI.getAccountBalanceAtBlockHeight(address, height);
 
     if (response instanceof FlowAccessApi.AccessApiCallResponse.Success) {
         return ((FlowAccessApi.AccessApiCallResponse.Success<Long>) response).getData();
     } else {
         FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) response;
-        throw new RuntimeException(errorResponse.getMessage(), errorResponse.getThrowable());
+        throw new AccountBalanceException("Failed to retrieve balance at block height " + height, errorResponse.getMessage(), errorResponse.getThrowable());
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public long getBalanceAtBlockHeight(FlowAddress address, long height) {
FlowAccessApi.AccessApiCallResponse<Long> response = accessAPI.getAccountBalanceAtBlockHeight(address, height);
if (response instanceof FlowAccessApi.AccessApiCallResponse.Success) {
return ((FlowAccessApi.AccessApiCallResponse.Success<Long>) response).getData();
} else {
FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) response;
throw new RuntimeException(errorResponse.getMessage(), errorResponse.getThrowable());
}
}
/**
* Retrieves the account balance at a specific block height.
*
* @param address The Flow address to query the balance for
* @param height The block height to query at
* @return The account balance
* @throws AccountBalanceException if the balance retrieval fails
* @throws IllegalArgumentException if address is null or height is negative
*/
public long getBalanceAtBlockHeight(FlowAddress address, long height) {
if (address == null) {
throw new IllegalArgumentException("address cannot be null");
}
if (height < 0) {
throw new IllegalArgumentException("block height cannot be negative");
}
FlowAccessApi.AccessApiCallResponse<Long> response = accessAPI.getAccountBalanceAtBlockHeight(address, height);
if (response instanceof FlowAccessApi.AccessApiCallResponse.Success) {
return ((FlowAccessApi.AccessApiCallResponse.Success<Long>) response).getData();
} else {
FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) response;
throw new AccountBalanceException("Failed to retrieve balance at block height " + height, errorResponse.getMessage(), errorResponse.getThrowable());
}
}

}
77 changes: 77 additions & 0 deletions ...a/org/onflow/examples/java/getAccountBalance/GetAccountBalanceAccessAPIConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package org.onflow.examples.java.getAccountBalance;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.onflow.flow.common.test.FlowEmulatorProjectTest;
import org.onflow.flow.common.test.FlowServiceAccountCredentials;
import org.onflow.flow.common.test.FlowTestClient;
import org.onflow.flow.common.test.TestAccount;
import org.onflow.flow.sdk.FlowAccessApi;
import org.onflow.flow.sdk.FlowAddress;
import org.onflow.flow.sdk.FlowBlock;

@FlowEmulatorProjectTest(flowJsonLocation = "../flow/flow.json")
public class GetAccountBalanceAccessAPIConnectorTest {

@FlowTestClient
private FlowAccessApi accessAPI;

@FlowServiceAccountCredentials
private TestAccount serviceAccount;

private GetAccountBalanceAccessAPIConnector balanceAPIConnector;

@BeforeEach
public void setup() {
balanceAPIConnector = new GetAccountBalanceAccessAPIConnector(accessAPI);
}

@Test
public void testCanFetchBalanceAtLatestBlock() {
FlowAddress address = serviceAccount.getFlowAddress();
long balance = balanceAPIConnector.getBalanceAtLatestBlock(address);

Assertions.assertTrue(balance >= 0, "Balance at the latest block should be non-negative");
}

@Test
public void testCanFetchBalanceAtSpecificBlockHeight() {
FlowAddress address = serviceAccount.getFlowAddress();

FlowAccessApi.AccessApiCallResponse<FlowBlock> latestBlockResponse = accessAPI.getLatestBlock(true);

if (latestBlockResponse instanceof FlowAccessApi.AccessApiCallResponse.Success) {
FlowBlock latestBlock = ((FlowAccessApi.AccessApiCallResponse.Success<FlowBlock>) latestBlockResponse).getData();
long blockHeight = latestBlock.getHeight();
long balanceAtHeight = balanceAPIConnector.getBalanceAtBlockHeight(address, blockHeight);

Assertions.assertTrue(balanceAtHeight >= 0, "Balance at specific block height should be non-negative");
} else {
FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) latestBlockResponse;
Assertions.fail("Failed to fetch the latest block: " + errorResponse.getMessage());
}
}
Comment on lines +38 to +54
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider refactoring duplicate error handling logic.

The error handling code for getLatestBlock is duplicated across test methods. Consider extracting it into a helper method.

private FlowBlock getLatestBlockOrFail() {
    FlowAccessApi.AccessApiCallResponse<FlowBlock> response = accessAPI.getLatestBlock(true);
    if (response instanceof FlowAccessApi.AccessApiCallResponse.Success) {
        return ((FlowAccessApi.AccessApiCallResponse.Success<FlowBlock>) response).getData();
    }
    FlowAccessApi.AccessApiCallResponse.Error errorResponse = 
        (FlowAccessApi.AccessApiCallResponse.Error) response;
    throw new AssertionError("Failed to fetch the latest block: " + errorResponse.getMessage());
}

Then use it in your tests:

@Test
public void testCanFetchBalanceAtSpecificBlockHeight() {
    FlowAddress address = serviceAccount.getFlowAddress();
    FlowBlock latestBlock = getLatestBlockOrFail();
    long blockHeight = latestBlock.getHeight();
    long balanceAtHeight = balanceAPIConnector.getBalanceAtBlockHeight(address, blockHeight);
    
    Assertions.assertTrue(balanceAtHeight >= 0, 
        String.format("Balance at height %d should be non-negative, but got: %d", 
            blockHeight, balanceAtHeight));
}

Also applies to: 56-76


@Test
public void testBalancesAtLatestBlockAndSpecificHeightShouldMatch() {
FlowAddress address = serviceAccount.getFlowAddress();

long balanceAtLatest = balanceAPIConnector.getBalanceAtLatestBlock(address);
FlowAccessApi.AccessApiCallResponse<FlowBlock> latestBlockResponse = accessAPI.getLatestBlock(true);

if (latestBlockResponse instanceof FlowAccessApi.AccessApiCallResponse.Success) {
FlowBlock latestBlock = ((FlowAccessApi.AccessApiCallResponse.Success<FlowBlock>) latestBlockResponse).getData();
long blockHeight = latestBlock.getHeight();

// Fetch balance at the same block height
long balanceAtHeight = balanceAPIConnector.getBalanceAtBlockHeight(address, blockHeight);
Comment on lines +67 to +68
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the implementation found, I can now provide the final response:

Add validation for negative block height parameter

The current implementation of getBalanceAtBlockHeight in GetAccountBalanceAccessAPIConnector does not validate the block height parameter. You should add a check at the beginning of the method to ensure the height parameter is non-negative:

if (height < 0) {
    throw new IllegalArgumentException("Block height cannot be negative");
}
🔗 Analysis chain

Add validation for block height parameter.

The getBalanceAtBlockHeight call should validate that the block height is non-negative before making the API call.

Let me check the actual implementation of the getBalanceAtBlockHeight method to verify if any validation exists.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if parameter validation exists in the connector implementation
# Expected: Should find parameter validation in the connector class

# Search for the connector class implementation
echo "Searching for GetAccountBalanceAccessAPIConnector implementation..."
rg -l "class GetAccountBalanceAccessAPIConnector"

# Search for parameter validation in the getBalanceAtBlockHeight method
echo "Checking for parameter validation..."
ast-grep --pattern 'getBalanceAtBlockHeight($address, $height) {
  if ($height < 0) {
    $$$
  }
  $$$
}'

Length of output: 848

Script:

#!/bin/bash
# Search for the getBalanceAtBlockHeight method implementation in the connector class
echo "Searching for getBalanceAtBlockHeight implementation..."
rg -A 10 "getBalanceAtBlockHeight" java-example/src/main/java/org/onflow/examples/java/getAccountBalance/GetAccountBalanceAccessAPIConnector.java

Length of output: 892


// Ensure balances match
Assertions.assertEquals(balanceAtLatest, balanceAtHeight, "Balance at latest block and specific block height should match");
} else {
FlowAccessApi.AccessApiCallResponse.Error errorResponse = (FlowAccessApi.AccessApiCallResponse.Error) latestBlockResponse;
Assertions.fail("Failed to fetch the latest block: " + errorResponse.getMessage());
}
}
}
19 changes: 19 additions & 0 deletions ...otlin/org/onflow/examples/kotlin/getAccountBalance/GetAccountBalanceAccessAPIConnector.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.onflow.examples.kotlin.getAccountBalance

import org.onflow.flow.sdk.*

internal class GetAccountBalanceAccessAPIConnector(
private val accessAPI: FlowAccessApi
) {
fun getBalanceAtLatestBlock(address: FlowAddress): Long =
when (val response = accessAPI.getAccountBalanceAtLatestBlock(address)) {
is FlowAccessApi.AccessApiCallResponse.Success -> response.data
is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable)
}
Comment on lines +8 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling and documentation

While the basic functionality is correct, consider these improvements:

  1. Create a custom exception type instead of using generic Exception
  2. Add KDoc documentation describing possible exceptions
  3. Add null safety for error message handling
+/** 
+ * Retrieves account balance at the latest block.
+ * @param address The Flow address to query
+ * @return The account balance as Long
+ * @throws FlowBalanceException if the API call fails
+ */
 fun getBalanceAtLatestBlock(address: FlowAddress): Long =
     when (val response = accessAPI.getAccountBalanceAtLatestBlock(address)) {
         is FlowAccessApi.AccessApiCallResponse.Success -> response.data
-        is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable)
+        is FlowAccessApi.AccessApiCallResponse.Error -> throw FlowBalanceException(
+            response.message ?: "Unknown error occurred",
+            response.throwable
+        )
     }

+class FlowBalanceException(message: String, cause: Throwable?) : Exception(message, cause)

Committable suggestion was skipped due to low confidence.


fun getBalanceAtBlockHeight(address: FlowAddress, height: Long): Long =
when (val response = accessAPI.getAccountBalanceAtBlockHeight(address, height)) {
is FlowAccessApi.AccessApiCallResponse.Success -> response.data
is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable)
}
Comment on lines +14 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add parameter validation and consistent error handling

The method should validate the height parameter and use consistent error handling with getBalanceAtLatestBlock.

+/** 
+ * Retrieves account balance at specified block height.
+ * @param address The Flow address to query
+ * @param height The block height (must be positive)
+ * @return The account balance as Long
+ * @throws FlowBalanceException if the API call fails or if height is invalid
+ */
 fun getBalanceAtBlockHeight(address: FlowAddress, height: Long): Long =
+    when {
+        height < 0 -> throw FlowBalanceException("Block height must be non-negative", null)
+        else ->
     when (val response = accessAPI.getAccountBalanceAtBlockHeight(address, height)) {
         is FlowAccessApi.AccessApiCallResponse.Success -> response.data
-        is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable)
+        is FlowAccessApi.AccessApiCallResponse.Error -> throw FlowBalanceException(
+            response.message ?: "Unknown error occurred",
+            response.throwable
+        )
     }
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun getBalanceAtBlockHeight(address: FlowAddress, height: Long): Long =
when (val response = accessAPI.getAccountBalanceAtBlockHeight(address, height)) {
is FlowAccessApi.AccessApiCallResponse.Success -> response.data
is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable)
}
/**
* Retrieves account balance at specified block height.
* @param address The Flow address to query
* @param height The block height (must be positive)
* @return The account balance as Long
* @throws FlowBalanceException if the API call fails or if height is invalid
*/
fun getBalanceAtBlockHeight(address: FlowAddress, height: Long): Long =
when {
height < 0 -> throw FlowBalanceException("Block height must be non-negative", null)
else ->
when (val response = accessAPI.getAccountBalanceAtBlockHeight(address, height)) {
is FlowAccessApi.AccessApiCallResponse.Success -> response.data
is FlowAccessApi.AccessApiCallResponse.Error -> throw FlowBalanceException(
response.message ?: "Unknown error occurred",
response.throwable
)
}
}

}
73 changes: 73 additions & 0 deletions ...n/org/onflow/examples/kotlin/getAccountBalance/GetAccountBalanceAccessAPIConnectorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package org.onflow.examples.kotlin.getAccountBalance

import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.onflow.flow.common.test.FlowEmulatorProjectTest
import org.onflow.flow.common.test.FlowServiceAccountCredentials
import org.onflow.flow.common.test.FlowTestClient
import org.onflow.flow.common.test.TestAccount
import org.onflow.flow.sdk.FlowAccessApi

@FlowEmulatorProjectTest(flowJsonLocation = "../flow/flow.json")
internal class GetAccountBalanceAccessAPIConnectorTest {
@FlowServiceAccountCredentials
lateinit var serviceAccount: TestAccount

@FlowTestClient
lateinit var accessAPI: FlowAccessApi

private lateinit var balanceAPIConnector: GetAccountBalanceAccessAPIConnector

@BeforeEach
fun setup() {
balanceAPIConnector = GetAccountBalanceAccessAPIConnector(accessAPI)
}

@Test
fun `Can fetch account balance at the latest block`() {
val address = serviceAccount.flowAddress
val balance = balanceAPIConnector.getBalanceAtLatestBlock(address)

Assertions.assertNotNull(balance, "Balance should not be null")
Assertions.assertTrue(balance >= 0, "Balance should be non-negative")
}
Comment on lines +27 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for getBalanceAtLatestBlock.

The test should handle potential errors from getBalanceAtLatestBlock similar to how getLatestBlock errors are handled in other tests.

 @Test
 fun `Can fetch account balance at the latest block`() {
     val address = serviceAccount.flowAddress
-    val balance = balanceAPIConnector.getBalanceAtLatestBlock(address)
-
-    Assertions.assertNotNull(balance, "Balance should not be null")
-    Assertions.assertTrue(balance >= 0, "Balance should be non-negative")
+    when (val result = balanceAPIConnector.getBalanceAtLatestBlock(address)) {
+        is FlowAccessApi.AccessApiCallResponse.Success -> {
+            val balance = result.data
+            Assertions.assertNotNull(balance, "Balance should not be null")
+            Assertions.assertTrue(balance >= 0, "Balance should be non-negative")
+        }
+        is FlowAccessApi.AccessApiCallResponse.Error -> 
+            Assertions.fail("Failed to retrieve balance: ${result.message}")
+    }
 }

Committable suggestion was skipped due to low confidence.


@Test
fun `Can fetch account balance at a specific block height`() {
val address = serviceAccount.flowAddress
val latestBlock = accessAPI.getLatestBlock(true) // Fetch the latest sealed block

when (latestBlock) {
is FlowAccessApi.AccessApiCallResponse.Success -> {
val balanceAtHeight = balanceAPIConnector.getBalanceAtBlockHeight(address, latestBlock.data.height)

Assertions.assertNotNull(balanceAtHeight, "Balance at specific block height should not be null")
Assertions.assertTrue(balanceAtHeight >= 0, "Balance at specific block height should be non-negative")
}
is FlowAccessApi.AccessApiCallResponse.Error -> Assertions.fail("Failed to retrieve the latest block: ${latestBlock.message}")
}
}

@Test
fun `Balances at the latest block and specific block height should match`() {
val address = serviceAccount.flowAddress

// Fetch balance at latest block
val balanceAtLatest = balanceAPIConnector.getBalanceAtLatestBlock(address)

// Fetch latest block height
val latestBlock = accessAPI.getLatestBlock(true)
when (latestBlock) {
is FlowAccessApi.AccessApiCallResponse.Success -> {
val blockHeight = latestBlock.data.height

// Fetch balance at the same block height
val balanceAtHeight = balanceAPIConnector.getBalanceAtBlockHeight(address, blockHeight)

Assertions.assertEquals(balanceAtLatest, balanceAtHeight, "Balance at latest block and specific block height should match")
}
is FlowAccessApi.AccessApiCallResponse.Error -> Assertions.fail("Failed to retrieve the latest block: ${latestBlock.message}")
}
}
}
Comment on lines +1 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add test cases for error scenarios.

The test suite would benefit from additional test cases covering error scenarios:

  • Invalid addresses
  • Non-existent block heights
  • Network errors

Example test case to add:

@Test
fun `Should handle invalid block height gracefully`() {
    val address = serviceAccount.flowAddress
    val invalidHeight = -1L
    
    val result = balanceAPIConnector.getBalanceAtBlockHeight(address, invalidHeight)
    
    when (result) {
        is FlowAccessApi.AccessApiCallResponse.Error -> 
            Assertions.assertTrue(result.message.contains("invalid block height"))
        is FlowAccessApi.AccessApiCallResponse.Success -> 
            Assertions.fail("Expected error for invalid block height")
    }
}

69 changes: 69 additions & 0 deletions sdk/src/intTest/org/onflow/flow/sdk/transaction/TransactionIntegrationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,75 @@ class TransactionIntegrationTest {
assertThat(blockHeader.height).isEqualTo(latestBlock.height)
}

@Test
fun `Can get account balance at latest block`() {
val address = serviceAccount.flowAddress

val balanceResponse = try {
handleResult(
accessAPI.getAccountBalanceAtLatestBlock(address),
"Failed to get account balance at latest block"
)
} catch (e: Exception) {
fail("Failed to retrieve account balance at latest block: ${e.message}")
}

assertThat(balanceResponse).isNotNull

val account = try {
handleResult(
accessAPI.getAccountAtLatestBlock(address),
"Failed to get account at latest block"
)
} catch (e: Exception) {
fail("Failed to retrieve account at latest block: ${e.message}")
}

val normalizedBalance = balanceResponse / 100_000_000L

assertThat(normalizedBalance).isEqualTo(account.balance.toBigInteger().longValueExact())
}

@Test
fun `Can get account balance at block height`() {
val address = serviceAccount.flowAddress

val latestBlock = try {
handleResult(
accessAPI.getLatestBlock(true),
"Failed to get latest block"
)
} catch (e: Exception) {
fail("Failed to retrieve latest block: ${e.message}")
}

val height = latestBlock.height

val balanceResponse = try {
handleResult(
accessAPI.getAccountBalanceAtBlockHeight(address, height),
"Failed to get account balance at block height"
)
} catch (e: Exception) {
fail("Failed to retrieve account balance at block height: ${e.message}")
}

assertThat(balanceResponse).isNotNull

val account = try {
handleResult(
accessAPI.getAccountByBlockHeight(address, height),
"Failed to get account by block height"
)
} catch (e: Exception) {
fail("Failed to retrieve account by block height: ${e.message}")
}

val normalizedBalance = balanceResponse / 100_000_000L

assertThat(normalizedBalance).isEqualTo(account.balance.toBigInteger().longValueExact())
}
Comment on lines +173 to +211
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error case tests and extracting common logic.

  1. The test only covers the happy path. Consider adding tests for:

    • Invalid block heights
    • Non-existent accounts
    • Network errors

  2. The normalization logic is duplicated. Consider extracting it to a helper method:

+ private fun normalizeFlowBalance(rawBalance: Long): Long =
+     rawBalance / FLOW_TOKEN_DECIMALS

     @Test
     fun `Can get account balance at block height`() {
         // ...
-        val normalizedBalance = balanceResponse / 100_000_000L
+        val normalizedBalance = normalizeFlowBalance(balanceResponse)

Committable suggestion was skipped due to low confidence.


@Test
fun `Can get latest block`() {
val latestBlock = try {
Expand Down
4 changes: 4 additions & 0 deletions sdk/src/main/kotlin/org/onflow/flow/sdk/AsyncFlowAccessApi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ interface AsyncFlowAccessApi {

fun getLatestBlock(sealed: Boolean = true): CompletableFuture<FlowAccessApi.AccessApiCallResponse<FlowBlock>>

fun getAccountBalanceAtLatestBlock(address: FlowAddress): CompletableFuture<FlowAccessApi.AccessApiCallResponse<Long>>

fun getAccountBalanceAtBlockHeight(address: FlowAddress, height: Long): CompletableFuture<FlowAccessApi.AccessApiCallResponse<Long>>

fun getBlockById(id: FlowId): CompletableFuture<FlowAccessApi.AccessApiCallResponse<FlowBlock?>>

fun getBlockByHeight(height: Long): CompletableFuture<FlowAccessApi.AccessApiCallResponse<FlowBlock?>>
Expand Down
4 changes: 4 additions & 0 deletions sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ interface FlowAccessApi {

fun getLatestBlock(sealed: Boolean = true): AccessApiCallResponse<FlowBlock>

fun getAccountBalanceAtLatestBlock(address: FlowAddress): AccessApiCallResponse<Long>

fun getAccountBalanceAtBlockHeight(address: FlowAddress, height: Long): AccessApiCallResponse<Long>

fun getBlockById(id: FlowId): AccessApiCallResponse<FlowBlock>

fun getBlockByHeight(height: Long): AccessApiCallResponse<FlowBlock>
Expand Down
51 changes: 51 additions & 0 deletions sdk/src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,57 @@ class AsyncFlowAccessApiImpl(
}
}

override fun getAccountBalanceAtLatestBlock(address: FlowAddress): CompletableFuture<FlowAccessApi.AccessApiCallResponse<Long>> {
return try {
completableFuture(
try {
api.getAccountBalanceAtLatestBlock(
Access.GetAccountBalanceAtLatestBlockRequest
.newBuilder()
.setAddress(address.byteStringValue)
.build()
)
} catch (e: Exception) {
return CompletableFuture.completedFuture(FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at latest block", e))
}
).handle { response, ex ->
if (ex != null) {
FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at latest block", ex)
} else {
FlowAccessApi.AccessApiCallResponse.Success(response.balance)
}
}
} catch (e: Exception) {
CompletableFuture.completedFuture(FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at latest block", e))
}
}

override fun getAccountBalanceAtBlockHeight(address: FlowAddress, height: Long): CompletableFuture<FlowAccessApi.AccessApiCallResponse<Long>> {
return try {
completableFuture(
try {
api.getAccountBalanceAtBlockHeight(
Access.GetAccountBalanceAtBlockHeightRequest
.newBuilder()
.setAddress(address.byteStringValue)
.setBlockHeight(height)
.build()
)
} catch (e: Exception) {
return CompletableFuture.completedFuture(FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at block height", e))
}
).handle { response, ex ->
if (ex != null) {
FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at block height", ex)
} else {
FlowAccessApi.AccessApiCallResponse.Success(response.balance)
}
}
} catch (e: Exception) {
CompletableFuture.completedFuture(FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at block height", e))
}
}

override fun getBlockById(id: FlowId): CompletableFuture<FlowAccessApi.AccessApiCallResponse<FlowBlock?>> {
return try {
completableFuture(
Expand Down
27 changes: 27 additions & 0 deletions sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,33 @@ class FlowAccessApiImpl(
FlowAccessApi.AccessApiCallResponse.Error("Failed to get latest block", e)
}

override fun getAccountBalanceAtLatestBlock(address: FlowAddress): FlowAccessApi.AccessApiCallResponse<Long> =
try {
val ret = api.getAccountBalanceAtLatestBlock(
Access.GetAccountBalanceAtLatestBlockRequest
.newBuilder()
.setAddress(address.byteStringValue)
.build()
)
FlowAccessApi.AccessApiCallResponse.Success(ret.balance)
} catch (e: Exception) {
FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at latest block", e)
}

override fun getAccountBalanceAtBlockHeight(address: FlowAddress, height: Long): FlowAccessApi.AccessApiCallResponse<Long> =
try {
val ret = api.getAccountBalanceAtBlockHeight(
Access.GetAccountBalanceAtBlockHeightRequest
.newBuilder()
.setAddress(address.byteStringValue)
.setBlockHeight(height)
.build()
)
FlowAccessApi.AccessApiCallResponse.Success(ret.balance)
} catch (e: Exception) {
FlowAccessApi.AccessApiCallResponse.Error("Failed to get account balance at block height", e)
}

override fun getBlockById(id: FlowId): FlowAccessApi.AccessApiCallResponse<FlowBlock> =
try {
val ret = api.getBlockByID(
Expand Down
Loading
Loading