Details

Reviewers

tobias_ruck
Fabien

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

This code creates Bitcoin Cash addresses using CashAddress format, with features like:

Hash encoding for address types (P2PKH, P2SH)

Error handling during address creation

Supports multiple address types for flexibility

Validates address format for correctness

Test Plan

Please set BUILD_DIR env to export BUILD_DIR="/path/to/build_dir
UNIX: ./contrib/teamcity/build-configurations.py build-bitcoinsuite-chronik-client

Diff Detail

Repository

rABC Bitcoin ABC

Branch

address

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 33106
Build 65696: Build Diff	ecash-lib-integration-tests · ecash-agora-integration-tests · chronik-client-integration-tests · build-explorer · build-chronik-plugins · build-chronik
Build 65695: arc lint + arc unit

Event Timeline

hazzarust created this revision.Sun, Apr 13, 14:03

Owners added a reviewer: Restricted Owners Package.Sun, Apr 13, 14:03

Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Apr 13, 14:03

hazzarust requested review of this revision.Sun, Apr 13, 14:03

Harbormaster completed remote builds in B32953: Diff 53464.Sun, Apr 13, 14:12

Build Bitcoin ABC Diffs / Diff Testing (chronik-client-integration-tests) failed.

Tail of the build log:

      data: "abc"
    }
  }
}
time_first_seen: 1744553504
size: 178
])
2025-04-13T14:11:45.066000Z TestFramework (INFO): Stopping nodes
2025-04-13T14:11:45.169000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20250413_141143/setup_scripts/chronik-client_plugins_0
2025-04-13T14:11:45.169000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20250413_141143/setup_scripts/chronik-client_plugins_0/test_framework.log
2025-04-13T14:11:45.169000Z TestFramework (ERROR): 
2025-04-13T14:11:45.169000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20250413_141143/setup_scripts/chronik-client_plugins_0' to consolidate all logs
2025-04-13T14:11:45.170000Z TestFramework (ERROR): 
2025-04-13T14:11:45.170000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2025-04-13T14:11:45.170000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2025-04-13T14:11:45.170000Z TestFramework (ERROR): 
Running Unit Tests for Test Framework Modules
[1msetup_scripts/chronik-client_plugins.py[0m started
[1msetup_scripts/chronik-client_plugins.py[0m failed, Duration: 2 s

[1mstdout:[0m

[1mstderr:[0m

[1m
TEST                                    | STATUS    | DURATION

[0m[0;31msetup_scripts/chronik-client_plugins.py | ✖ Failed  | 2 s
[0m[0;31m[1m
ALL                                     | ✖ Failed  | 2 s (accumulated) 
[0m[0mRuntime: 2 s

Test runner for chronik-client_plugins completed with code 1
----------------------|---------|----------|---------|---------|------------------------------------
File                  | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                  
----------------------|---------|----------|---------|---------|------------------------------------
All files             |   31.16 |    12.31 |   28.39 |   31.13 |                                    
 chronik-client       |     100 |      100 |     100 |     100 |                                    
  index.ts            |     100 |      100 |     100 |     100 |                                    
 chronik-client/proto |   23.53 |     9.16 |   17.05 |   23.74 |                                    
  chronik.ts          |   23.53 |     9.16 |   17.05 |   23.74 | ...4,6520-6523,6529-6571,6607-6616 
 chronik-client/src   |   67.21 |    49.69 |   65.41 |   66.48 |                                    
  ChronikClient.ts    |   60.38 |    48.69 |   63.63 |   60.38 | ...6,1357-1365,1373-1438,1446-1451 
  failoverProxy.ts    |   79.43 |    58.06 |   69.23 |   78.84 | ...286-289,292,301,308,312,317,321 
  hex.ts              |   89.47 |       50 |      75 |   87.87 | 58,66-68                           
  validation.ts       |    75.6 |       40 |      75 |   72.97 | 17,21,33,38-49,62-63               
----------------------|---------|----------|---------|---------|------------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='988']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='3170']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='258']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='2095']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='161']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='567']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='976']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='3135']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/chronik-client-integration-tests-junit.xml': No such file or directory
Build chronik-client-integration-tests failed with exit code 1

Please don't review

Removed serial and deserial, removed useless lifetimes, changed from_le_bytes to from_be_hex and thus handled array -> str conversion

tobias_ruck requested changes to this revision.Mon, Apr 14, 09:36

tobias_ruck added a subscriber: tobias_ruck.

tobias_ruck added inline comments.

modules/bitcoinsuite-chronik-client/Cargo.toml
37 ↗	(On Diff #53465)	remove unused dependency
modules/bitcoinsuite-chronik-client/src/address.rs
1 ↗	(On Diff #53465)	this should be in bitcoinsuite-core
64 ↗	(On Diff #53465)	move this into the FromStr::parse below
115 ↗	(On Diff #53465)	you can remove this, only makes sense with Cow
124 ↗	(On Diff #53465)	same here
149 ↗	(On Diff #53465)	avoid the `_` prefix—it indicates that it might be unused, but we don't want that. instead either just remove the `_` prefix or put them all into a `mod inner {}` for special safekeeping
269 ↗	(On Diff #53465)

This revision now requires changes to proceed.Mon, Apr 14, 09:36

Harbormaster completed remote builds in B32954: Diff 53465.Mon, Apr 14, 09:49

Added address.rs to chronik/bitcoinsuite-core, added documentation, changed return types, cleaned the code (kachow)

Harbormaster completed remote builds in B33104: Diff 53691.Fri, Apr 25, 10:44

Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Tail of the build log:

   Doc-tests chronik-bridge

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-db

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-http

running 1 test
test chronik/chronik-http/src/protobuf.rs - protobuf::Protobuf (line 29) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.38s

   Doc-tests chronik-indexer

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-common

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-impl

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-proto

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-util

running 2 tests
test chronik/chronik-util/src/log.rs - log::log (line 65) ... ignored
test chronik/chronik-util/src/log.rs - log::log_chronik (line 87) ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 0.00s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.

Tail of the build log:

   Doc-tests chronik-bridge

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-db

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-http

running 1 test
test chronik/chronik-http/src/protobuf.rs - protobuf::Protobuf (line 29) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.36s

   Doc-tests chronik-indexer

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-common

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-impl

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-proto

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-util

running 2 tests
test chronik/chronik-util/src/log.rs - log::log (line 65) ... ignored
test chronik/chronik-util/src/log.rs - log::log_chronik (line 87) ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 0.00s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1

reviewed anyway

chronik/bitcoinsuite-core/src/address.rs
1 ↗	(On Diff #53691)	copyright missing, you plan on stealing this? smh
6–8 ↗	(On Diff #53691)	merge those into one `use`
57 ↗	(On Diff #53691)	add an extra newline after each for legibility
401 ↗	(On Diff #53691)	I think this would be cleaner
423 ↗	(On Diff #53691)

This revision now requires changes to proceed.Fri, Apr 25, 10:58

Made changes

Harbormaster completed remote builds in B33105: Diff 53692.Fri, Apr 25, 11:34

Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.

Tail of the build log:

    Checking rustls-pki-types v1.10.0
    Checking untrusted v0.9.0
    Checking utf16_iter v1.0.5
    Checking write16 v1.0.0
    Checking utf8_iter v1.0.4
    Checking parking v2.2.1
   Compiling fastrand v2.2.0
   Compiling which v4.4.2
    Checking event-listener v5.3.1
    Checking stable-eyre v0.2.2
   Compiling petgraph v0.6.5
    Checking abc-rust-error v0.1.0 (/work/chronik/abc-rust-error)
   Compiling tempfile v3.14.0
   Compiling cxx v1.0.133
    Checking bitcoinsuite-core v0.1.0 (/work/chronik/bitcoinsuite-core)
   Compiling prost-types v0.11.9
    Checking form_urlencoded v1.2.1
    Checking versions v6.3.2
    Checking httparse v1.9.5
    Checking icu_normalizer v1.5.0
    Checking tower-service v0.3.3
    Checking subtle v2.6.1
    Checking bimap v0.6.3
    Checking option-ext v0.2.0
    Checking dirs-sys v0.4.1
    Checking idna_adapter v1.2.0
   Compiling cxx-build v1.0.133
    Checking num-traits v0.2.19
    Checking event-listener-strategy v0.5.3
   Compiling pyo3-ffi v0.23.3
   Compiling pyo3-macros-backend v0.23.3
    Checking spin v0.9.8
    Checking hash32 v0.2.1
   Compiling prost-build v0.11.9
    Checking tracing-core v0.1.33
    Checking tower-layer v0.3.3
error: using `clone` on type `ShaRmd160` which implements the `Copy` trait
   --> chronik/bitcoinsuite-core/src/address.rs:170:49
    |
170 |         Self::from_hash(prefix, self.addr_type, self.hash.clone())
    |                                                 ^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `self.hash`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
    = note: `-D clippy::clone-on-copy` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::clone_on_copy)]`

error: this expression creates a reference which is immediately dereferenced by the compiler
   --> chronik/bitcoinsuite-core/src/address.rs:199:56
    |
199 |         let (hash, addr_type, prefix) = from_cash_addr(&s)?;
    |                                                        ^^ help: change this to: `s`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
    = note: `-D clippy::needless-borrow` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::needless_borrow)]`

error: could not compile `bitcoinsuite-core` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1

Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Tail of the build log:

   Doc-tests chronik-bridge

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-db

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-http

running 1 test
test chronik/chronik-http/src/protobuf.rs - protobuf::Protobuf (line 29) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.37s

   Doc-tests chronik-indexer

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-common

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-impl

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-proto

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-util

running 2 tests
test chronik/chronik-util/src/log.rs - log::log (line 65) ... ignored
test chronik/chronik-util/src/log.rs - log::log_chronik (line 87) ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 0.00s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

Roasted by linter, updating

Harbormaster completed remote builds in B33106: Diff 53693.Fri, Apr 25, 12:27

Did you notice that we're not bch ? It would be nice to use appropriated defaults in the doc and the code...

Also please add support for the testnet addresses.

chronik/bitcoinsuite-core/src/address.rs
385	bad name, and bad type. You'd better use AddressType

This revision now requires changes to proceed.Fri, Apr 25, 14:41

D17886 [chronik] Add failover_proxy to bitcoinsuite-chronik-client

·Reviewers: Restricted Owners Package, Restricted Project, Fabien, tobias_ruck

Tue, Apr 15, 10:26

Author: hazzarust

D17859 [chronik] Add failover_proxy to bitcoinsuite-chronik-client

·Reviewers: Restricted Owners Package, Restricted Project

Wed, Apr 2, 14:14

Author: hazzarust

[chronik] Add address.rs from givelotus/bitcoinsuite into bitcoinsuite-chronik-client
Needs RevisionPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 53693

Cargo.lock

chronik/bitcoinsuite-core/Cargo.toml

chronik/bitcoinsuite-core/src/address.rs

chronik/bitcoinsuite-core/src/lib.rs

[chronik] Add address.rs from givelotus/bitcoinsuite into bitcoinsuite-chronik-clientNeeds RevisionPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 53693

Cargo.lock

chronik/bitcoinsuite-core/Cargo.toml

chronik/bitcoinsuite-core/src/address.rs

chronik/bitcoinsuite-core/src/lib.rs

[chronik] Add address.rs from givelotus/bitcoinsuite into bitcoinsuite-chronik-client
Needs RevisionPublic
Actions

Revision Contents
Changeset List