Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
There are docs for all the main features in the read me here https://github.com/aptos-labs/japtos |
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive Java SDK documentation for Aptos, including installation instructions, quickstart guides, account management, transaction building, and extensive code examples.
- Adds Java SDK overview page with installation and basic usage examples
- Includes quickstart guide with step-by-step instructions for a simple token transfer
- Provides detailed documentation on account management including Ed25519, multi-signature, and HD wallets
- Documents transaction building, signing, simulation, and submission workflows
- Adds comprehensive code examples covering common use cases
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/content/nav/en.ts | Adds Java SDK navigation label |
| src/content/docs/build/sdks/java-sdk/quickstart.mdx | New quickstart guide with complete APT transfer example |
| src/content/docs/build/sdks/java-sdk/java-examples.mdx | New examples page with multiple use cases and patterns |
| src/content/docs/build/sdks/java-sdk/building-transactions.mdx | New transaction building guide with detailed explanations |
| src/content/docs/build/sdks/java-sdk/account.mdx | New account management guide covering various account types |
| src/content/docs/build/sdks/java-sdk.mdx | New Java SDK overview page with features and basic examples |
| src/content/docs/build/sdks.mdx | Adds Java SDK link to main SDKs page |
| astro.sidebar.ts | Adds Java SDK section to sidebar navigation |
Comments suppressed due to low confidence (1)
src/content/docs/build/sdks/java-sdk/account.mdx:1
- This code accesses nested JSON properties without null checks. While this may work in controlled examples, consider adding a note that production code should include null checking or error handling when accessing JSON properties, as the resource might not exist or have an unexpected structure.
---
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| System.out.println("Mnemonic: " + mnemonic); | ||
|
|
||
| // Derive multiple accounts from same mnemonic |
There was a problem hiding this comment.
The BIP44 derivation path format uses hardened derivation (indicated by the apostrophe). Consider adding a brief note explaining what the numbers represent: m / purpose' / coin_type' / account' / change' / address_index'. This would help developers understand how to modify the path for deriving multiple accounts.
| // Derive multiple accounts from same mnemonic | |
| // Derive multiple accounts from same mnemonic | |
| // BIP44 derivation path format: m / purpose' / coin_type' / account' / change' / address_index' | |
| // For Aptos, purpose is 44', coin_type is 637', account' is the account number, change' is 0' (external), address_index' is 0' |
| return resource.getAsJsonObject("coin") | ||
| .get("value") | ||
| .getAsLong(); |
There was a problem hiding this comment.
The getBalance method chains JSON access operations without null safety. Consider adding a note in the surrounding documentation that this simplified approach assumes the account is funded and the resource exists. Production code should handle cases where the resource might not exist.
| return resource.getAsJsonObject("coin") | ||
| .get("value") | ||
| .getAsLong(); |
There was a problem hiding this comment.
Similar to other examples, this JSON property access lacks null safety. Since this is in an examples file, consider adding a comment within the code or in the surrounding text noting that error handling has been omitted for brevity but should be included in production code.
rrigoni
left a comment
There was a problem hiding this comment.
We are moving the library to maven central repo and configuration will change (packageid), lets hold until IT configures the maven central repo so I can push to it.
|
@gregnazario , it is published, please update the maven artifactId in the docs to the following: https://central.sonatype.com/artifact/io.github.aptos-labs/japtos |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| ModuleId moduleId = new ModuleId( | ||
| AccountAddress.fromHex("0x1"), | ||
| new Identifier("coin") | ||
| ); | ||
|
|
||
| TransactionPayload payload = new EntryFunctionPayload( | ||
| moduleId, | ||
| new Identifier("transfer"), | ||
| Arrays.asList(new TypeTag.Struct(new StructTag( | ||
| AccountAddress.fromHex("0x1"), | ||
| new Identifier("aptos_coin"), | ||
| new Identifier("AptosCoin"), | ||
| Arrays.asList() | ||
| ))), | ||
| Arrays.asList( | ||
| new TransactionArgument.AccountAddress(bob.getAccountAddress()), | ||
| new TransactionArgument.U64(1_000_000L) // 0.01 APT | ||
| ) | ||
| ); |
There was a problem hiding this comment.
The function name should be "transfer" but this example uses 0x1::coin::transfer which requires a coin type parameter (as shown in the type arguments). However, the type argument provided doesn't match the simpler pattern used elsewhere in the documentation. Consider using 0x1::aptos_account::transfer instead (with empty type arguments) for consistency with the quickstart and other examples, or if keeping 0x1::coin::transfer, the implementation should match the pattern shown in building-transactions.mdx lines 112-133.
| SimulationResult simulation = client.simulateTransaction(rawTx, sender); | ||
|
|
||
| System.out.println("Simulation results:"); | ||
| System.out.println("Success: " + simulation.isSuccess()); | ||
| System.out.println("Gas used: " + simulation.getGasUsed()); | ||
| System.out.println("VM status: " + simulation.getVmStatus()); |
There was a problem hiding this comment.
The SimulationResult class and its methods (isSuccess(), getGasUsed(), getVmStatus()) are used but not imported. Add the appropriate import statement for SimulationResult or clarify the package/class name needed for this functionality.
| ## Best Practices | ||
|
|
||
| 1. **Always simulate transactions** before submitting to check for errors | ||
| 2. **Set appropriate gas limits** - not too low (fails) or too high (wastes limits) |
There was a problem hiding this comment.
Unclear phrasing: "not too low (fails) or too high (wastes limits)" is confusing. The parenthetical "(wastes limits)" doesn't make sense. Consider rephrasing to "not too low (transaction fails) or too high (wastes gas allowance)" or "not too low (causes failure) or too high (unnecessarily high maximum fee)".
| 2. **Set appropriate gas limits** - not too low (fails) or too high (wastes limits) | |
| 2. **Set appropriate gas limits** - not too low (transaction fails) or too high (wastes gas allowance) |
|
|
||
| - Learn about [account management](/build/sdks/java-sdk/account) including multi-signature and HD wallets | ||
| - Explore [building transactions](/build/sdks/java-sdk/building-transactions) in depth | ||
| - Check out more [examples](/build/sdks/java-sdk/java-examples) in the SDK repository |
There was a problem hiding this comment.
Misleading link text. The text says "Check out more examples in the SDK repository" but the link points to /build/sdks/java-sdk/java-examples which is a documentation page, not the GitHub repository. Either change the text to "Check out more code examples" or change the link to point to the actual repository at https://github.com/aptos-labs/japtos/tree/main/src/test.
| - Check out more [examples](/build/sdks/java-sdk/java-examples) in the SDK repository | |
| - Check out more [code examples](/build/sdks/java-sdk/java-examples) |
| new Identifier("coin") | ||
| ); | ||
|
|
||
| TransactionPayload payload = new EntryFunctionPayload( | ||
| moduleId, | ||
| new Identifier("transfer"), | ||
| Arrays.asList(new TypeTag.Struct(new StructTag( | ||
| AccountAddress.fromHex("0x1"), | ||
| new Identifier("aptos_coin"), | ||
| new Identifier("AptosCoin"), | ||
| Arrays.asList() | ||
| ))), |
There was a problem hiding this comment.
The module name should be "aptos_account" instead of "coin" for the transfer function shown in the quickstart and other examples. The 0x1::coin::transfer function requires a type parameter, while 0x1::aptos_account::transfer is the simpler transfer function that doesn't require type parameters and is more appropriate for a basic example.
| new Identifier("coin") | |
| ); | |
| TransactionPayload payload = new EntryFunctionPayload( | |
| moduleId, | |
| new Identifier("transfer"), | |
| Arrays.asList(new TypeTag.Struct(new StructTag( | |
| AccountAddress.fromHex("0x1"), | |
| new Identifier("aptos_coin"), | |
| new Identifier("AptosCoin"), | |
| Arrays.asList() | |
| ))), | |
| new Identifier("aptos_account") | |
| ); | |
| TransactionPayload payload = new EntryFunctionPayload( | |
| moduleId, | |
| new Identifier("transfer"), | |
| Arrays.asList(), // No type arguments needed for aptos_account::transfer |
| ```java | ||
| import com.aptoslabs.japtos.account.MultiEd25519Account; | ||
| import com.aptoslabs.japtos.core.crypto.Ed25519PrivateKey; | ||
| import java.util.Arrays; | ||
|
|
||
| public class MultiSigTransfer { | ||
| public static void main(String[] args) throws Exception { | ||
| AptosConfig config = AptosConfig.builder() | ||
| .network(AptosConfig.Network.DEVNET) | ||
| .build(); | ||
| AptosClient client = new AptosClient(config); |
There was a problem hiding this comment.
Missing import statements. This multi-sig example uses AptosConfig, AptosClient, and other classes without importing them. Add the necessary imports for completeness.
|
|
||
| <LinkCard href="/build/sdks/java-sdk/building-transactions" title="Building Transactions" description="Learn how to build, sign, and submit transactions" /> | ||
|
|
||
| <LinkCard href="/build/sdks/java-sdk/java-examples" title="Examples" description="Explore comprehensive examples in the SDK repository" target="_blank" /> |
There was a problem hiding this comment.
The link text says "Examples" but the description says "Explore comprehensive examples in the SDK repository". However, this link points to /build/sdks/java-sdk/java-examples which is part of the documentation site, not the SDK repository. Either remove target="_blank" since it's an internal link, or change the href to point to the actual GitHub repository examples (e.g., https://github.com/aptos-labs/japtos/tree/main/src/test).
| <LinkCard href="/build/sdks/java-sdk/java-examples" title="Examples" description="Explore comprehensive examples in the SDK repository" target="_blank" /> | |
| <LinkCard href="https://github.com/aptos-labs/japtos/tree/main/src/test" title="Examples" description="Explore comprehensive examples in the SDK repository" target="_blank" /> |
| SimulationResult simulation = client.simulateTransaction(rawTx, sender); | ||
| System.out.println("Estimated gas: " + simulation.getGasUsed()); | ||
|
|
||
| if (!simulation.isSuccess()) { | ||
| throw new RuntimeException("Simulation failed: " + simulation.getVmStatus()); | ||
| } |
There was a problem hiding this comment.
Similarly, SimulationResult needs to be imported in this complete example.
| public class BalanceCheck { | ||
| public static void main(String[] args) throws Exception { | ||
| AptosConfig config = AptosConfig.builder() | ||
| .network(AptosConfig.Network.DEVNET) | ||
| .build(); | ||
| AptosClient client = new AptosClient(config); | ||
|
|
||
| // Create and fund account | ||
| Ed25519Account account = Ed25519Account.generate(); | ||
| client.fundAccount(account.getAccountAddress(), 50_000_000); |
There was a problem hiding this comment.
Missing import statements. This balance check example uses AptosConfig, AptosClient, and Ed25519Account without importing them.
| ``` | ||
|
|
||
| :::caution | ||
| Account funding via the faucet is only available on **Devnet** and **Localnet**. For Mainnet and Testnet, you'll need to fund accounts through other means (exchanges, faucets, etc.). |
There was a problem hiding this comment.
Incorrect information about faucet availability. The statement says "For Mainnet and Testnet, you'll need to fund accounts through other means (exchanges, faucets, etc.)" which is confusing because it says Testnet doesn't have faucet access but then mentions "faucets" as an alternative means. According to /network/faucet.mdx, Testnet does have a faucet available. The comment should clarify: "Account funding via the SDK's fundAccount() method is only available on Devnet and Localnet. For Testnet, use the Testnet Faucet. For Mainnet, you'll need to acquire APT through exchanges or other means."
| Account funding via the faucet is only available on **Devnet** and **Localnet**. For Mainnet and Testnet, you'll need to fund accounts through other means (exchanges, faucets, etc.). | |
| Account funding via the SDK's `fundAccount()` method is only available on **Devnet** and **Localnet**. For **Testnet**, use the [Testnet Faucet](/network/faucet). For **Mainnet**, you'll need to acquire APT through exchanges or other means. |
AI generated some docs for the Japtos SDK
Have not had time to go and review it too much, so please take a look at it.