Skip to content

Feat/upgrade web3dart#47

Open
arman-garousi-bitazza wants to merge 2 commits intomasterfrom
feat/upgrade-web3dart
Open

Feat/upgrade web3dart#47
arman-garousi-bitazza wants to merge 2 commits intomasterfrom
feat/upgrade-web3dart

Conversation

@arman-garousi-bitazza
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Member

@LiorAgnin LiorAgnin left a comment

Choose a reason for hiding this comment

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

PR #47 Verification Report — Score: 42/100

Verdict: REQUEST_CHANGES

Hi @arman-garousi-bitazza, thank you for working on this upgrade! The structural approach is solid — all typechain files are consistently updated. However, there are several issues that need to be addressed before this can be merged.


Critical Issues (must fix)

1. EthereumAddress not re-exported from barrel
lib/userop.dart exports package:web3dart/web3dart.dart but does NOT export package:wallet/wallet.dart. Since EthereumAddress moved to the wallet package in web3dart v3, consumers who relied on import 'package:userop/userop.dart' to access EthereumAddress will get compilation errors.

Fix: Add export 'package:wallet/wallet.dart'; to lib/userop.dart.

2. Silent EthereumAddress.toString() behavior change
In web3dart v2, EthereumAddress.toString() returned checksummed hex WITH 0x prefix (e.g., 0xAbC123...). In the wallet package, it returns eip55Without0x (e.g., AbC123...). This is a silent breaking change — it won't cause compile errors but will break:

  • IUserOperation.opToJson() serialization
  • Any RPC call or bundler interaction that expects 0x-prefixed addresses
  • Address comparisons in downstream code

Fix: Audit all .toString() call sites and consider adding a wrapper or using .eip55With0x explicitly.

3. web3dart_builders git dependency fails
The dev dependency changed from ^0.1.2 (pub.dev) to an SSH git URL (git@github.com:AstroxNetwork/web3dart_builders.git). This:

  • Fails to clone without SSH credentials (Permission denied (publickey))
  • Makes the package unpublishable to pub.dev
  • Blocks dependency resolution, compilation, and tests

Fix: Use an HTTPS URL or wait for a published version.


Medium Issues (should fix)

4. Removed properties without migration docs

  • EthereumAddress.addressBytes → use .value
  • EthereumAddress.hex → use .with0x, .without0x, .eip55With0x, or .eip55Without0x

5. Removed sub-libraries
package:web3dart/crypto.dart and package:web3dart/credentials.dart no longer exist in v3.0.2.

6. Version bump too small
0.3.1 → 0.3.2 is a patch bump, but this PR introduces breaking changes (new required dependency, moved types, changed behavior). Should be at least 0.4.0 per semver.

7. dependency_overrides for pointycastle
Adding pointycastle: ^4.0.0 as a dependency override can mask version conflicts.


Minor Issues

  • CHANGELOG.md not updated for 0.3.2
  • Unnecessary import 'package:wallet/wallet.dart' in gas_price.dart (no wallet symbols used)
  • Dead commented-out imports in 3 example files (// import 'package:web3dart/crypto.dart')

What's Good

  • All 10 typechain .g.dart files are consistently and correctly updated (_i1=web3dart, _i2=wallet, _i3=typed_data)
  • No stale _i1.EthereumAddress references remain
  • The addressBytes → value change in kernel.dart is structurally correct
  • The approach of using package:wallet for the EthereumAddress type is the right direction

Recommended Priority Order

  1. Fix web3dart_builders dependency (use HTTPS or published version)
  2. Add export 'package:wallet/wallet.dart' to lib/userop.dart
  3. Audit EthereumAddress.toString() call sites
  4. Bump version to 0.4.0
  5. Document breaking changes and update CHANGELOG.md

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