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.

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

Docs and nodejs #2582

Closed
wants to merge 0 commits into from
Closed

Docs and nodejs #2582

wants to merge 0 commits into from

Conversation

dirvine
Copy link
Member

@dirvine dirvine commented Dec 31, 2024

Node.js Integration, Documentation Enhancement, and AttoTokens Fix

Overview

This PR introduces comprehensive documentation, Node.js integration capabilities, and fixes a critical issue with AttoTokens parsing. The changes aim to improve developer experience, expand platform accessibility, and enhance system reliability.

Key Changes

Documentation Enhancements

  • Added MkDocs-based documentation system with comprehensive structure
  • Implemented GitHub Actions workflow for automatic documentation deployment
  • Created detailed guides for:
    • Client modes and usage
    • Data storage and management
    • EVM integration
    • Local development setup
    • Testing procedures
    • Payment handling
  • Added API documentation for:
    • ant-node
    • autonomi-client
    • blsttc
    • self-encryption

Node.js Integration

  • Added Node.js bindings for core functionality:
    • Linked List operations
    • Pointer management
    • Vault interactions
    • Wallet operations
  • Included TypeScript definitions for improved type safety
  • Added comprehensive tests for Node.js bindings
  • Created local testnet action for testing Node.js integrations
  • Added Node.js-specific documentation and examples

Bug Fixes

  • Fixed AttoTokens parsing to properly handle large values:
    • Added validation for excessive values before multiplication
    • Updated precision handling for decimal places
    • Enhanced error handling for edge cases
    • Added comprehensive test coverage for token parsing

Infrastructure Improvements

  • Updated .gitignore to handle Node.js and documentation artifacts
  • Added GitHub Actions workflow for documentation
  • Updated dependency management in Cargo.lock

Testing

  • All Rust tests are passing, including the enhanced AttoTokens tests
  • Added Node.js integration tests
  • Documentation build and deployment verified
  • Local testnet functionality verified

Impact

These changes significantly improve the project's accessibility and reliability:

  • Developers can now integrate with the system using Node.js
  • Documentation is more comprehensive and easier to maintain
  • Token handling is more robust and reliable
  • Testing infrastructure is more comprehensive

Breaking Changes

None. All changes are backward compatible.

Future Work

  • Consider adding more Node.js examples
  • Expand documentation with more use cases
  • Add performance benchmarks for Node.js bindings

Checklist

  • Documentation updated
  • Tests added and passing
  • Code follows project style guidelines
  • CI/CD pipelines updated
  • No breaking changes introduced

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

devskim found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Member

@grumbach grumbach left a comment

Choose a reason for hiding this comment

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

Excellent PR with many good contributions like docs and tests. Some doubts though about client mode. Left some comments. Noted CI has many failures

tokio::fs::write(&cache_path, "invalid json content").await?;

// Create a new cache store - it should handle the corruption gracefully
Copy link
Member

Choose a reason for hiding this comment

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

I find these comments to be very helpful when reading tests. Any specific reason to remove those?

@@ -0,0 +1 @@
Subproject commit 600912168508b206c51f974440d2316b06618a98
Copy link
Member

Choose a reason for hiding this comment

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

What is this file needed for?

Copy link
Member Author

Choose a reason for hiding this comment

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

This seemed to be a ci file, but I had a git issue so could not tell, so I left it in place in case it was used

header.kind
);
Err(NetworkError::RecordKindMismatch(RecordKind::Chunk).into())
ClientMode::ReadOnly => Err(PutError::NoWallet),
Copy link
Member

Choose a reason for hiding this comment

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

What does the mode protect us against? I'm wondering if it would be simpler without them?

}

/// Check if the client has write access (i.e. has a wallet)
pub fn check_write_access(&self) -> Result<(), PutError> {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like all this complexity can be removed if we get rid of the mode

Copy link
Member

Choose a reason for hiding this comment

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

Why is this a git submodule?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants