From e93f848498dbb325118cd3fbeceacfd4f84cacd1 Mon Sep 17 00:00:00 2001 From: Kirk Haines Date: Sat, 3 Jun 2023 21:04:02 -0600 Subject: [PATCH] Re-enable ameba; cleanup code, and fix a couple bugs. --- .github/workflows/ci.yml | 6 ++---- README.md | 4 +++- benchmark/src/benchmark.cr | 24 ++++++++++++++---------- shard.yml | 8 ++++---- spec/decoder_specs.cr | 6 ++++-- spec/encoder_specs.cr | 18 +++++++++++------- src/base58/decoder.cr | 14 +++++++------- src/base58/decoder_check.cr | 5 ++--- src/base58/encoder.cr | 2 +- src/base58/encoder_check.cr | 2 -- src/base58/extensions/benchmark.cr | 6 +++--- src/base58/extensions/string.cr | 4 ++-- src/base58/ss58.cr | 4 ++-- 13 files changed, 55 insertions(+), 48 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e6957d..71360c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,5 @@ jobs: run: crystal spec -t -s - name: Linting run: crystal tool format --check -# - name: Run Ameba -# run: bin/ameba - - name: Build docs - run: crystal docs + - name: Run Ameba + run: bin/ameba diff --git a/README.md b/README.md index eb34b87..62bb8ed 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,6 @@ -![Base58 CI](https://img.shields.io/github/actions/workflow/status/wyhaines/base58.cr/ci.yml?branch=main&logo=GitHub) +[![Base58 CI](https://github.com/wyhaines/base58.cr/actions/workflows/ci.yml/badge.svg)](https://github.com/wyhaines/base58.cr/actions/workflows/ci.yml) +[![Base58 Build Docs](https://github.com/wyhaines/base58.cr/actions/workflows/build_docs.yml/badge.svg)](https://github.com/wyhaines/base58.cr/actions/workflows/build_docs.yml) + [![GitHub release](https://img.shields.io/github/release/wyhaines/base58.cr.svg?style=for-the-badge)](https://github.com/wyhaines/base58.cr/releases) ![GitHub commits since latest release (by SemVer)](https://img.shields.io/github/commits-since/wyhaines/base58.cr/latest?style=for-the-badge) diff --git a/benchmark/src/benchmark.cr b/benchmark/src/benchmark.cr index 3bc4e91..6425764 100644 --- a/benchmark/src/benchmark.cr +++ b/benchmark/src/benchmark.cr @@ -18,9 +18,11 @@ module MyBenchmark SliceBuffer = Slice(UInt8).new(256) PRNG = Random.new + # ameba:disable Style/ConstantNames Encoded_16777215 = ::Base58.encode(16777215) - Encoded_ffffff = ::Base58.encode("\xff\xff\xff") - MoneroAddress = "12c09d10f3c5f580ddd0765063d9246007f45ef025a76c7d117fe4e811fa78f3959c66f7487c1bef43c64ee0ace763116456666a389eea3b693cd7670c3515a0c043794fbf".hexbytes + # ameba:disable Style/ConstantNames + Encoded_ffffff = ::Base58.encode("\xff\xff\xff") + MoneroAddress = "12c09d10f3c5f580ddd0765063d9246007f45ef025a76c7d117fe4e811fa78f3959c66f7487c1bef43c64ee0ace763116456666a389eea3b693cd7670c3515a0c043794fbf".hexbytes def self.run Benchmark.ips(warmup: 0.5, calculation: 1.25) do |ips| @@ -118,16 +120,18 @@ module MyBenchmark def self.run_rust_benchmarks(ips) ips.separator "Rust Benchmarks".colorize.green.bold ips.separator("(if Rust is installed and available...)".colorize.light_yellow.dim) do |max_label| - begin - Dir.cd("./rustbench") - Process.run("cargo", args: {"bench"}) do |io| - io.output.gets_to_end.lines.select(&.index("time:")).each do |line| - label = line.split("time:").first.strip - data = line.scan(/(\d+\.\d+)\s+(\w+)/)[1][0] - puts "#{"".rjust(max_label.not_nil! - label.size)}#{label} (#{data.colorize.light_red})" + if max_label + begin + Dir.cd("./rustbench") + Process.run("cargo", args: {"bench"}) do |io| + io.output.gets_to_end.lines.select(&.index("time:")).each do |line| + label = line.split("time:").first.strip + data = line.scan(/(\d+\.\d+)\s+(\w+)/)[1][0] + puts "#{"".rjust(max_label - label.size)}#{label} (#{data.colorize.light_red})" + end end + rescue Exception end - rescue Exception end end end diff --git a/shard.yml b/shard.yml index 274b0e9..71e6c81 100644 --- a/shard.yml +++ b/shard.yml @@ -8,7 +8,7 @@ crystal: ">1.6.0" license: "Apache 2.0" -#development_dependencies: -# ameba: -# github: crystal-ameba/ameba -# version: ~> 1.3.0 +development_dependencies: + ameba: + github: crystal-ameba/ameba + version: ~> 1.4 diff --git a/spec/decoder_specs.cr b/spec/decoder_specs.cr index 1e06374..f0f3347 100644 --- a/spec/decoder_specs.cr +++ b/spec/decoder_specs.cr @@ -73,6 +73,7 @@ describe Base58::Decoder do should_be_the_same, len = Base58.decode( Base58.encode(testcase["hex"].as(String).hexbytes, into: Slice(UInt8)), into: slice) + should_be_the_same.should eq slice slice[..(len - 1)].hexstring.should eq testcase["hex"] end end @@ -185,6 +186,7 @@ describe Base58::Decoder do buffer = Base58.encode(testcase["hex"].as(String).hexbytes, into: Slice(UInt8)) stringbuffer = StringBuffer.new(32) should_be_the_same_buffer = Base58.decode(buffer, into: stringbuffer) + stringbuffer.buffer.should be should_be_the_same_buffer (stringbuffer.buffer == String.new(testcase["hex"].as(String).hexbytes)).should be_true end end @@ -309,7 +311,7 @@ describe Base58::Decoder do it "can decode encoded strings to a new StringBuffer, with Base58Check" do TestData::Strings.select { |tc| tc["check_prefix"]? }.select { |tc| tc["alphabet"] == Base58::Alphabet::Bitcoin }.each do |testcase| - res = Base58.decode( + Base58.decode( Base58.encode( String.new(testcase["hex"].as(String).hexbytes), check: Base58::Check.new(testcase["check_prefix"].as(String)) @@ -516,7 +518,7 @@ describe Base58::Decoder do it "can decode encoded strings to a new StringBuffer, with Base58Check" do TestData::Strings.select { |tc| tc["check_prefix"]? }.select { |tc| tc["alphabet"] == Base58::Alphabet::Bitcoin }.each do |testcase| - res = Base58.decode( + Base58.decode( Base58.encode( String.new(testcase["hex"].as(String).hexbytes), check: Base58::Check.new(type: :CB58, prefix: testcase["check_prefix"].as(String)) diff --git a/spec/encoder_specs.cr b/spec/encoder_specs.cr index a7489de..ef0daa9 100644 --- a/spec/encoder_specs.cr +++ b/spec/encoder_specs.cr @@ -15,11 +15,12 @@ describe Base58::Encoder do end it "encodes static arrays to strings with the default (Bitcoin) alphabet" do - testcase = TestData::Strings.reject { |tc| tc["check_prefix"]? }.select { |tc| tc["alphabet"] == Base58::Alphabet::Bitcoin }.first - stat_ary = StaticArray(UInt8, 12).new(0) - bytes = testcase["hex"].as(String).hexbytes - stat_ary.to_unsafe.copy_from(bytes.to_unsafe, bytes.size) - Base58.encode(stat_ary).should eq testcase["string"] + if testcase = TestData::Strings.reject { |tc| tc["check_prefix"]? }.find { |tc| tc["alphabet"] == Base58::Alphabet::Bitcoin } + stat_ary = StaticArray(UInt8, 12).new(0) + bytes = testcase["hex"].as(String).hexbytes + stat_ary.to_unsafe.copy_from(bytes.to_unsafe, bytes.size) + Base58.encode(stat_ary).should eq testcase["string"] + end end it "encodes stringbuffers to strings with the default (Bitcoin) alphabet" do @@ -81,7 +82,8 @@ describe Base58::Encoder do TestData::Strings.reject { |tc| tc["check_prefix"]? }.select { |tc| tc["alphabet"] == Base58::Alphabet::Bitcoin }.each do |testcase| bytes = testcase["hex"].as(String).hexbytes len = testcase["string"].as(String).size - ptr, len = Base58.encode(bytes, into: Pointer) + ptr, new_len = Base58.encode(bytes, into: Pointer) + len.should eq new_len Slice.new(ptr, len).should eq testcase["string"].as(String).to_slice end end @@ -121,6 +123,7 @@ describe Base58::Encoder do len = testcase["string"].as(String).size buffer = StaticArray(UInt8, 256).new(0) should_be_the_same_buffer, newlen = Base58.encode(bytes, into: buffer) # But it really isn't, since StaticArrays are passed by Copy. + len.should eq newlen Slice.new(should_be_the_same_buffer.to_unsafe, newlen).should eq testcase["string"].as(String).to_slice end end @@ -174,8 +177,9 @@ describe Base58::Encoder do it "uses the Encoder.into() syntax to encode strings to a raw buffer" do TestData::Strings.reject { |tc| tc["check_prefix"]? }.select { |tc| tc["alphabet"] == Base58::Alphabet::Bitcoin }.each do |testcase| bytes = testcase["hex"].as(String).hexbytes - len = testcase["string"].as(String).size + test_len = testcase["string"].as(String).size ptr, len = Base58::Encoder.into(Pointer).encode(bytes).as(Tuple(Pointer(UInt8), Int32)) + test_len.should eq len Slice.new(ptr, len.as(Int32)).should eq testcase["string"].as(String).to_slice end end diff --git a/src/base58/decoder.cr b/src/base58/decoder.cr index ed6b601..bc4bc01 100644 --- a/src/base58/decoder.cr +++ b/src/base58/decoder.cr @@ -33,8 +33,8 @@ module Base58 @[AlwaysInline] def self.decode(value : Array(UInt8), into = String, alphabet : Alphabet.class = Alphabet::Bitcoin) pointer = GC.malloc_atomic(value.size).as(UInt8*) - value.each_with_index do |value, i| - pointer[i] = value + value.each_with_index do |inner_value, i| + pointer[i] = inner_value end decode(pointer, value.size, into, alphabet) end @@ -42,8 +42,8 @@ module Base58 @[AlwaysInline] def self.decode(value : Array(Char), into = String, alphabet : Alphabet.class = Alphabet::Bitcoin) pointer = GC.malloc_atomic(value.size).as(UInt8*) - value.each_with_index do |value, i| - pointer[i] = value.ord.to_u8 + value.each_with_index do |inner_value, i| + pointer[i] = inner_value.ord.to_u8 end decode(pointer, value.size, into, alphabet) end @@ -110,7 +110,6 @@ module Base58 def self.decode(value : Pointer(UInt8), size : Int32, into : String, alphabet : Alphabet.class = Alphabet::Bitcoin) original_size = into.bytesize buffer_size = original_size + size + 1 - bigptr = GC.malloc_atomic(buffer_size * 2).as(UInt8*) String.new(buffer_size) do |ptr| ptr.copy_from(into.to_slice.to_unsafe, original_size) _, final_size = decode_into_pointer(value, ptr + original_size, size, alphabet) @@ -211,6 +210,7 @@ module Base58 @[AlwaysInline] def self.primary_decoding(value : Pointer(UInt8), pointer : Pointer(UInt8), size : Int, index : Int, pointer_index : Int, alphabet : Alphabet.class) + # ameba:disable Lint/ShadowedArgument pointer_index = 0 while index < size val = alphabet.inverse(value[index]).to_u16 @@ -262,7 +262,7 @@ module Base58 index = 0 pointer_index = 0 - index, pointer_index = primary_decoding(value, pointer, size, index, pointer_index, alphabet) + _, pointer_index = primary_decoding(value, pointer, size, index, pointer_index, alphabet) pointer_index = zero_padding(value, pointer, size, pointer_index, alphabet[0]) reverse_decoding(pointer, pointer_index) @@ -275,7 +275,7 @@ module Base58 aggregate_pointer_index = 0 ntimes, remainder = size.divmod(11) iterations = (remainder.zero? ? ntimes : ntimes + 1) - iterations.times do |nth_iteration| + iterations.times do |_| index = aggregate_index pointer_index = aggregate_pointer_index target_size = index + 11 diff --git a/src/base58/decoder_check.cr b/src/base58/decoder_check.cr index b3fb682..0442244 100644 --- a/src/base58/decoder_check.cr +++ b/src/base58/decoder_check.cr @@ -66,7 +66,6 @@ module Base58 def self.decode(value : Pointer(UInt8), size : Int32, check : Check, into : String, alphabet : Alphabet.class = Alphabet::Bitcoin) original_size = into.bytesize buffer_size = original_size + size + 1 - bigptr = GC.malloc_atomic(buffer_size * 2).as(UInt8*) String.new(buffer_size) do |ptr| ptr.copy_from(into.to_slice.to_unsafe, original_size) _, final_size = decode_into_pointer(value, ptr + original_size, size, check, alphabet) @@ -155,7 +154,7 @@ module Base58 @[AlwaysInline] def self.validate_checksum?(pointer, pointer_index, check) slice = Slice.new(pointer, pointer_index) - payload = slice[..-4] + # payload = slice[..-4] -- this is the payload, but we don't need it checksum = slice[-4..] case check.type @@ -187,7 +186,7 @@ module Base58 index = 0 pointer_index = 0 - index, pointer_index = primary_decoding(value, pointer, size, index, pointer_index, alphabet) + _, pointer_index = primary_decoding(value, pointer, size, index, pointer_index, alphabet) pointer_index = zero_padding(value, pointer, size, pointer_index, alphabet[0]) reverse_decoding(pointer, pointer_index) validate_checksum(pointer, pointer_index, check) diff --git a/src/base58/encoder.cr b/src/base58/encoder.cr index 6daf539..6f63e8f 100644 --- a/src/base58/encoder.cr +++ b/src/base58/encoder.cr @@ -436,7 +436,6 @@ module Base58 # Encodes the contents of a memory buffer, referenced by a Pointer(UInt8), into a newly allocated memory buffer. @[AlwaysInline] def self.encode_to_pointer(value : Pointer(UInt8), size : Int, alphabet : Alphabet.class = Alphabet::Bitcoin) : {Pointer(UInt8), Int32} - index = 0 buffer_size = SizeLookup[size]? || size * 2 ptr = GC.malloc_atomic(buffer_size).as(UInt8*) encode_into_pointer(value, ptr, size, alphabet) @@ -484,6 +483,7 @@ module Base58 # This encodes the contents of a memory buffer referenced by a Pointer(UInt8) into another existing memory buffer, for Monero. All Monero encoding eventually # ends up with this method. + # ameba:disable Metrics/CyclomaticComplexity private def self.encode_into_pointer(value : Pointer(UInt8), pointer : Pointer(UInt8), size : Int, alphabet : Alphabet::Monero.class) zer0 = alphabet[0] aggregate_index = 0 diff --git a/src/base58/encoder_check.cr b/src/base58/encoder_check.cr index 74c3955..380036f 100644 --- a/src/base58/encoder_check.cr +++ b/src/base58/encoder_check.cr @@ -263,7 +263,6 @@ module Base58 # Encodes the contents of a memory buffer, referenced by a Pointer(UInt8), into a newly allocated memory buffer, with checksumming. @[AlwaysInline] def self.encode_to_pointer(value : Pointer(UInt8), size : Int, check : Base58::Check, alphabet : Alphabet.class = Alphabet::Bitcoin) : {Pointer(UInt8), Int32} - index = 0 buffer_size = SizeLookup[size + check.prefix.bytesize + 4]? || (size + check.prefix.bytesize + 4) * 2 ptr = GC.malloc_atomic(buffer_size).as(UInt8*) encode_into_pointer(value, ptr, size, check, alphabet) @@ -291,7 +290,6 @@ module Base58 index = primary_encoding(vptr, pointer, vsize, index) end - not_finished_zero_padding = true { {prefix_slice.to_unsafe, check.prefix.bytesize}, {value, size}, {checksum_ptr, check.checksum_length} }.each do |vptr, vsize| index = zero_padding(vptr, pointer, vsize, index) end diff --git a/src/base58/extensions/benchmark.cr b/src/base58/extensions/benchmark.cr index baff6a8..70d75fb 100644 --- a/src/base58/extensions/benchmark.cr +++ b/src/base58/extensions/benchmark.cr @@ -57,7 +57,7 @@ module Benchmark end class Job - property fullscreen_report : Bool = false + property? fullscreen_report : Bool = false property scrollback : Int32 = 0 def initialize(calculation = 5, warmup = 2, interactive = STDOUT.tty?) @@ -73,7 +73,7 @@ module Benchmark def separator(label) : Benchmark::IPS::Entry self.fullscreen_report = true - item = Entry.new(label, ->(x : Int32) {}) + item = Entry.new(label, ->(_x : Int32) {}) item.separator = true @items << item @@ -174,7 +174,7 @@ module Benchmark end bytes += bytes_taken cycles += item.cycles - measurements << elapsed.not_nil! + measurements << elapsed if elapsed break if Time.monotonic >= target end diff --git a/src/base58/extensions/string.cr b/src/base58/extensions/string.cr index 76509cb..357c5e7 100644 --- a/src/base58/extensions/string.cr +++ b/src/base58/extensions/string.cr @@ -9,7 +9,7 @@ class String # of the string. It is used by the StringBuffer class to create a string which will later # mutated, with content inserted. def self.new(size : Int) - new(size) do |ptr| + new(size) do |_| {size, size} end end @@ -79,7 +79,7 @@ class StringBuffer # def initialize(@capacity : Int = 256) if capacity = @capacity - @buffer = String.new(capacity) do |ptr| + @buffer = String.new(capacity) do |_| {capacity, capacity} end end diff --git a/src/base58/ss58.cr b/src/base58/ss58.cr index 029c1a7..9919e16 100644 --- a/src/base58/ss58.cr +++ b/src/base58/ss58.cr @@ -184,7 +184,7 @@ module Base58 def self.decode_address(address, into : Slice(UInt8).class, format : Int? = nil) buffer = Base58.decode(address, into: Slice(UInt8)) - decoded_format, format_length = decode_and_validate_format(buffer, format) + _, format_length = decode_and_validate_format(buffer, format) buffer_size = buffer.size @@ -259,7 +259,7 @@ module Base58 checksum_length = checksum_length(buffer_size) Base58.decode(address, into: buffer) - format_length, decoded_format = decode_and_validate_format(buffer, format) + _, format_length = decode_and_validate_format(buffer, format) checksum = Slice.new(buffer + buffer_size - checksum_length, checksum_length) data = Slice.new(buffer + format_length, buffer_size - checksum_length - format_length)