This function is deprecated, stop using it in favour of
`Hash::from_byte_array`.
Patch only touches test code, I'm guessing that is why lint warnings
were no showing up.
Currently the `Hkdf` type does not derive any traits.
We would like to derive the common set of traits but there are a bunch
reasons we can't;
- Don't want to leak secrets in `Debug`.
- Don't want to enable timing attacks with Eq/Ord and friends.
For now just derive `Copy` and `Clone`. We will then implement `Debug`
manually.
The two main public macros can be used in function scope - prove it.
While we are at it prove that additional attributes are supported by
them both as well as visability keywords.
`hex/std` and `hex/alloc` should only be included if optional
dependency `hex` is enabled`.
Add `?` so it is only included if the optional feature `hex` is enabled.
Currently in order to release `hashes v1.0` we need to 1.0 `io` as well.
For multiple reasons, many out of our control, the `io` crate may not
stabalise any time soon.
Instead we can invert the dependency between the two crates.
This is an ingenious idea, props to Kixunil for coming up with it.
Notes
- `io` does not currently re-export the `hashes` crate.
- This work highlights that we cannot call `hash_reader` on a siphash.
- The `Hmac::hash_reader` uses the default key which may not be obvious.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
We would like it if two different pre-tagged engines were considered
different types so it is not possible to mix them up.
Add a new `sha256t::HashEngine<T>` where `T` is a tag the same as on
`sha256t::Hash<T>`.
This function is meant to be used in conjunction with the
`general_hash_type` macro but the `sha256t` module does not use that
macro.
Inline the function. Internal change only.
Now we have an associated const we can do away with the `engine` trait
method all together. Users can call `Hash<FooTag>::engine` instead. This
is better because its an API more similar to the other hash types and
therefor easier to discover and remember.
Instead of requiring users of the `Tag` trait to implement the `engine`
method we can have an associated const and provide an `engine` method
with a default implementation.
Use an associated const for the pre-tagged hash engine. Fro now keep the
`engine` trait method but have a default impl that returns the const. We
will remove it as a separate patch to assist review.
This macro is a maintenance burden. We would like to put a tag on the
hash engine but doing so would require breaking this macro anyway so
lets just delete it.
We are trying to make the `hashes` crate easier to read and also
possibly parseable by machines to see what is different where.
Move the test and bench code into separate files. Make special effort to
keep formatting as is so the diff is easier to review. We will run the
formatter in the next patch.
Internal change only.
There are five modules in `hashes` that implement cryptography (i.e.
have a `process_block` function). For each of them create a new
submodule called `crypto` and move the code there.
Code organisational refactor only, no logic changes.
In preparation for removing a bunch of macros move all the modules to
`<mod>/mod.rs`.
Do so by running the following shell:
```bash
for mod in hash160 ripemd160 sha1 sha256 sha256d sha256t \
sha384 sha512 sha512_256 siphash24 hkdf hmac; do
mkdir $mod
mv "$mod.rs" "$mod/mod.rs"
done
```
Internal change only.
The other hashes that require initial state (keys etc) both have an
`engine` function on the hash type because they cannot use the
`GeneralHash::engine` function.
Add an `engine` function to the `siphash24::Hash` type.
b98c489066 hashes: Move from_engine functions (Tobin C. Harding)
Pull request description:
In order to use the `general_hash_type` macro the must exist a standalone `from_engine` function. Currently this function is in different places in different modules. In an effort to make the `hashes` code easier to reason about put the functions right below the macro.
Code move only, no other changes.
ACKs for top commit:
Kixunil:
ACK b98c489066
apoelstra:
ACK b98c489066e8916a383099e5037e5a24832548ba; successfully ran local tests
Tree-SHA512: 8dfbf2b422d078d687708fa94a478ca597fae141f5c1f0a318a36152ca33f4760bb0545ab67523c558a8c3b8d258356975c5e357600d0ac980d473250a2af20e
18619a6d0b api: Run just check-api (Tobin C. Harding)
1eb8f1f9e0 Add a Hmac::engine function (Tobin C. Harding)
c352d376ed Do not implement Default for HmacEngine (Tobin C. Harding)
Pull request description:
The `HmacEngine` should be created using a key. Currently we are providing a `Default` impl that uses `&[]` as the key. This is, I believe, a hangover from when we had a `Default` trait bound somewhere else. It is incorrect and an API footgun - remove it.
Note this PR includes changes to the bench code in `hmac` that highlights the footgun - pity the poor user we even shot ourselves.
Patch 2 adds a constructor `Hmac::engine` and uses it in the bench code.
ACKs for top commit:
Kixunil:
ACK 18619a6d0b
apoelstra:
ACK 18619a6d0b0bca7b7e3603e260b254b4aae6cebf; successfully ran local tests
Tree-SHA512: c96640e7ffba52d5b13b76a6dd9e1381788efcf56ee76300c5111541466bab8018d2546bcecf237c42dbd82e9372a0e43e1ecec37147508e879365d92a4c1451
The `HmacEngine` cannot be constructed by way of the `GeneralHash` trait
method because it requires a key. However we can still add an inherent
function to the type to construct an engine.
Add the engine constructor and use it in bench code.
The `HmacEngine` should be created using a key. Currently we are
providing a `Default` impl that uses `&[]` as the key. This is, I
believe, a hangover from when we had a `Default` trait bound somewhere
else. It is incorrect and an API footgun - remove it.
In order to use the `general_hash_type` macro the must exist a
standalone `from_engine` function. Currently this function is in
different places in different modules. In an effort to make the `hashes`
code easier to reason about put the functions right below the macro.
Code move only, no other changes.
85e04315d5 Remove test_ prefix from unit tests (Tobin C. Harding)
Pull request description:
There is a loose convention in Rust to not use `test_` prefix. The reason being that `cargo test` outputs 'test <test name>' using the prefix makes the output stutter.
This patch smells a bit like code-churn but having the prefix in some places and not others is confusing to new contributors and is leading me to explain this many times now. Lets just fix it.
Remove the prefix unless doing so breaks the code.
ACKs for top commit:
shinghim:
ACK 85e04315d5
apoelstra:
ACK 85e04315d5eb90075ce55bf18fab8876a4583def; successfully ran local tests
Tree-SHA512: d90ae5ef75cc5e5a8f43f60819544f1a447f13cbe660ba71e84b8f27bfcc04a11d3afde0ed56e4eea5c73ebc3925024b800a1b995f73142cab892f97a414f14a
There is a loose convention in Rust to not use `test_` prefix. The
reason being that `cargo test` outputs 'test <test name>' using the
prefix makes the output stutter.
This patch smells a bit like code-churn but having the prefix in some
places and not others is confusing to new contributors and is leading me
to explain this many times now. Lets just fix it.
Remove the prefix unless doing so breaks the code.
Rust macros, while at times useful, are a maintenance nightmare. And
we have been bitten by calling macros from other crates multiple times
in the past.
In a push to just use less macros remove the usage of the
`impl_from_infallible` macro in all the leaf crates and just write the
code.
We have two macro definitions feature gated on `serde`. At some stage we
added the `doc(hidden)` attribute to one of them but forgot to add it to
the other. This technically makes our features non-additive. This macro
is "internal" so its unlikely that this is being used in the wild.
Add `doc(hidden)` to the `serde_impl` macro that is missing it.
Found by `cargo semver-checks` after recent upgrade to 0.38
We need to do a quick release because it turns out I was wrong in
thinking that making `hex` an optional dependency is not a breaking
change.
```
the package `bitcoin` depends on `bitcoin_hashes`, with features: `hex`
but `bitcoin_hashes` does not have these features. It has a required
dependency with that name, but only optional dependencies can be used as
features.
```
Add a changelog, bump the version, depend on the new version repo wide,
and update the lock files - like its our job.
To conform to Rust API guidelines examples should Examples use ?, not
try!, not unwrap (C-QUESTION-MARK).
Label the examples as `# Examples`.
Replace one `unwrap()` with `expect()` . The others don't technically
conform to the guidelines but are warranted.
The only reason we need `hex-conservative` is to parse strings and
format them as hex. For users that do not require this functionality we
can make the `hex-conservative` crate an optional dependency.
The `serde` feature requires `Display` so we enable `hex` from the
`serde` feature.
If `hex` feature is not enabled we still need to be able to debug so
provide `fmt::Debug` functionality by way of macros.
Close: #2654
For the `hashes` crate we would like to make `hex` an optional
dependency. In preparation for doing so do the following:
- Remove the trait bounds from `GeneralHash`
- Split the hex/string stuff out of `impl_bytelike_traits` into a
separate macro.
The `impl_bytelike_traits` macro is public and it is used in the
`hash_newtype` macro, also public.
Currently if a user calls the `hash_newtype` macro in a crate that
depends on `hashes` without the `serde` feature enabled and with no
`serde` dependency everything works. However if the user then adds a
dependency that happens to enable the `serde` feature in `hashes` their
build will blow up because `serde` code will start getting called from
the original crate's call to `hash_newtype`.
Pull the serde stuff out of `hash_newtype` and provide a macro to
implement it `impl_serde_for_newtype`.
1649b68589 Standardize wording to `constructs a new` (Jamil Lambert, PhD)
27f94d5540 Replace `creates` with `constructs` (Jamil Lambert, PhD)
Pull request description:
As discussed in issue #3575 there are various ways of saying a new object is created.
These have all be standardized to the agreed version.
Close#3575
ACKs for top commit:
apoelstra:
ACK 1649b68589834dfe9d5b63812da3e9f0e5930107; successfully ran local tests
tcharding:
ACK 1649b68589
Tree-SHA512: 0ed9b56819c95f1fc14da1e0fdbbe03c4af2d97a95ea6b56125f72913e8d832db5d2882d713ae139d00614e651f3834a4d72528bdf776231cceb6772bf2f9963
fe8ca21ec2 hashes: Duplicate impl_from_infallible (Tobin C. Harding)
7652d0ddfc hashes: Hide innards of FromSliceError (Tobin C. Harding)
5232bba62b hashes: Move FromSliceError to submodule (Tobin C. Harding)
Pull request description:
Hide the internals of the `hashes::FromSliceError`.
ACKs for top commit:
apoelstra:
ACK fe8ca21ec282cfe50e3f39b7f86c619d30d7f542; successfully ran local tests
Tree-SHA512: dac33777353dd81c8c86de331b2ab30d0a5268f2be7685f85405d29809ec36eeab31b0e71c9f09e820e06a93c3f05b7d675e5e729b780e8600b960cad4a02c77
There is a range of different wordings used in the docs of constructor
type functions.
Change all to start with `Constructs a new` or `Constructs an empty`.
In functions that act like constructors there is a mixture of the usage
of `creates` and `constructs`.
Replace all occurrences of `creates` with `constructs` in the first line
of docs of constructor like functions.
We don't want a dependency on `internals` for `hashes` unless its really
needed.
Duplicated the `impl_from_infallible` macro into `hashes::error` and
use it to implement from infallible for `FromSliceError`.
Add a public `error` module and move the one error for the `hashes`
crate into it.
Re-export at the crate root.
Done in preparation for adding an additional error type.
We have a couple of problems:
1. There are two macros currently for fmt stuff that do similar things,
`arr_newtype_fmt_impl` and `hex_fmt_impl` - the difference is not
immediately obvious, its the way that the byte array is iterated.
2. Our hash types are missing `AsRef<[u8; len]>` and `Borrow<[u8; len]>`.
Introduce a new macro and remove a bunch of other macros. Include
extensive docs but hide the macro from public docs because its not
really for consumers of the library.
The macro requires `$crate::hex` to point to `hex-conservative`.
Note the macro is pretty generic (as in general purpose), `hashes` might
not be the right home for it. Potentially a better place would be in
`hex` itself?
6aa8c2b023 Remove needless_borrows_for_generic_args (Tobin C. Harding)
Pull request description:
This has been fixed and we use nightly to lint so we have access to the merged fix.
ACKs for top commit:
apoelstra:
ACK 6aa8c2b023619534437043e76ec4252f27295875; successfully ran local tests
Tree-SHA512: 011677b6c1a2dc6c63097cafd1682fab5d9bdd94133b872bdd49699a55c80a01a8e6e5e844ae1cfe1ca9da47a2ba2ed6b910719b1ccbb06e60e22ecb01ec0efc
80b5d8b7b3 hashes: Add changelog for release 0.15.0 (Tobin C. Harding)
Pull request description:
The version has already been bumped on `master`.
In preparation for releasing `hashes v0.15.0` add a changelog entry.
ACKs for top commit:
apoelstra:
ACK 80b5d8b7b36ea97007f062971a732ec8a512cfa5; successfully ran local tests; again lots of stuff!
Tree-SHA512: ab1bbdf7df819c587dc905e4cdcda2232ede2a495a0e395e5d6f6bd730a7c3c0675e05f8ab616e5e6dee0a87836ab486d025f80608e73b713007391711ef15ba
This has been fixed and we use nightly to lint so we have access to the
merged fix.
Removing the attribute uncovers a bunch of real lint warnings, fix
them while we are at it.
We are about to release `bitcoin_hashes 0.15.0`, replace the TBD string
with the version number.
Requires changing `allow(deprecated_in_future)` attribute to
`allow(deprecated)` (in functions that are them self deprecated).
We have two files one for public macros and one for private macros. Move
the `engine_input_impl` macro to the private macros file.
Requires change to call sites because we do not have `use_macros`
attribute on the `internal_macros` file.
These three macros are solely provided to reduce code duplication, they
are only part of the public API because they are used by the "real"
public macro `hash_newtype`.
Roll the `serde_macros` module into `macros`, requires making `macros`
public but since it explicitly holds public macros this is reasonable.
Keep the original module and deprecate it.
The private and public modules are already grouped, add a line of
whitespace to make it _even_ more clear. Trivial I know, this patch got
smaller during rebase.
5ec17a2ee8 CI: Hobble WASM job (Tobin C. Harding)
Pull request description:
The WASM job is not working. `wasm-pack build` works fine but `wasm-pack test` doesn't compile.
Comment out the line that runs `wasm-pack test`.
ACKs for top commit:
apoelstra:
ACK 5ec17a2ee8c963d182777a9661b1bab786956bd4; successfully ran local tests; LGTM
Tree-SHA512: 5ea88bd99f625a8c669dc1dec02486dd549e9ac83d7670c6c7b3b25c86de3fc9a8dbc959dc39de92305dd5e254e0b8ed9424cae5e5941b66a6476ee433009fdd
1cb24c1f15 hashes:: Polish crate level rustdocs (Tobin C. Harding)
98691186dc hashes: Move engine functions (Tobin C. Harding)
12f261c009 hashes: Re-order from_byte_array (Tobin C. Harding)
c11587d60d hashes: Rename hash_type macro (Tobin C. Harding)
62617cf9ac hashes: Move from_engine function to other macro (Tobin C. Harding)
bb7dd2c479 hashes: Move DISPLAY_BACKWARD to top of impl block (Tobin C. Harding)
71013afe07 hashes: Put attribute under doc (Tobin C. Harding)
Pull request description:
`hashes 1.0.0` can't be far away, here is a quick polish (done while I waited for my car to get fixed).
Everything here is internal except stuff to docs. Note please I claim "internal change only" in a bunch of patches that do code moves but these effect the docs build because order is preserved in some instances.
ACKs for top commit:
apoelstra:
ACK 1cb24c1f150bc2d65d0be439f2005f41d95ad23c; successfully ran local tests
Tree-SHA512: 430c451afab8fc92fb4596bf2d4b36c086333fe72b3fe5858925b75597641b8c4f5e49f7643888fa19b675d3070ce9a3606623cd56bdba6cfc59e459fbdda440
The `sha256t` module is unique in that it implements its methods
manually (as call throughs) instead of using the macros. To make it more
clear what is implemented re-order the engine constructor and getter to
better mirror the layout in the macros.
Internal change only.
The `hashes` crate has a bunch of similar types defined by a bunch of
similar macros and impl blocks, all of which makes it difficult to tell
exactly what is implemented where. In an effort to make the code easier
to read order the `from_byte_array` constructor in the same place across
the crate. Note also we typically put constructors up the top, also
`from_byte_array` is the likely most used constructor so put it first.
FWIW I discovered this while polishing the HTML docs.
Internal change only.
Conceptually (and using traits) we split the hashes into "general"
hash types and more restricted hash types (`Hash`). Also we observe that
the `hash_type` macro defines all the inherent functions name
identically to the `GeneralHash` trait methods.
Rename the trait to describe better what it does.
Internal change only.
The `from_engine` function is associated with a general hash type but we
are defining it in the `hash_type` macro which holds nothing but
functions associated with the `Hash` trait. By "associated" I mean
methods on the type as opposed to trait methods.
In preparation for re-naming the `hash_type` macro move the
`from_engine` function there. Requires duplicating the code in the
`siphash` impl block, this is as expected because the `siphash` requires
a custom implementation of the general hashing functionality.
Internal change only.
We use `TBD` in our `deprecated` string and it was discovered that there
is an exception on this string so as not to warn because it is used
internally by the Rust language. However there is a special lint to
enable warnings, lets use it.
Add `#![warn(deprecated_in_future)]` to the coding conventions section
of all crates except `fuzz`.
We had an initial go at this but we didn't do the `Hash` trait method.
In order to do so we need to hack the serde code a fair bit, note the
public visitor types.
3b7ba4f977 Remove the SliceIndex implementation from hash types (Tobin C. Harding)
Pull request description:
If folk really want to index into a hash they can us `as_byte_array` then index that.
Includes a bump to the version number of `hashes` to `v0.15.0` - this is because otherwise `secp` won't build since we are breaking an API that is used in the current release of secp.
Fix: #3115
ACKs for top commit:
apoelstra:
ACK 3b7ba4f977 successfully ran local tests
Tree-SHA512: 0ba93268cd8133fe683183c5e39ab8b3bf25c15bfa5767d2934d67a5f6a0d2f65f6c9304952315fe8a33abfce488d810a8d28400a28facfb658879ed06acca63
`length` was changed to `bytes_hashed` and the type changed from `usize`
to `u64`.
The addition under the `hashes_fuzz` feature flag has been changed from
`usize` to `u64` to fix the fuzz error.
When `hashes_fuzz` is on the field `length` is refered to but it was
changed to `bytes_hashed` in a previous PR.
This has been fixed by changing `length` to `bytes_hashed`.
If folk really want to index into a hash they can us `as_byte_array`
then index that.
Includes a bump to the version number of `hashes` to `v0.15.0` - this
is because otherwise `secp` won't build since we are breaking an API
that is used in the current release of secp.
Fix: #3115
In preparation for releasing `io v0.2.0` bump the version number,
add a changelog entry, update the lock files, and depend on the new
version in all crates that depend on `io`.
be94c9e54b Rename Midstate::into_parts to Midstate::to_parts since it derives Copy (Shing Him Ng)
Pull request description:
This is a follow up to #3010Fixes#3079
ACKs for top commit:
tcharding:
ACK be94c9e54b
apoelstra:
ACK be94c9e54b successfully ran local tests
Tree-SHA512: 0d838a2064136d050d319116f6df3d598323b04a137e7bb7cb5f3f1a87d72ad1ee4d2f3b228a2f9d68e7ca117c0f922ef6551f783eb39c8db0db1188e4732c41
8bb0d3f667 Fix buggy cfg in rustdocs (Tobin C. Harding)
Pull request description:
In b9643bf3e9 we introduced an incorrect `cfg` attribute, that has just shown up, no clue why clippy only just presented me with this error now. Anywho, the current code is buggy and the rustdoc tests are never being run.
Fix `cfg` attribute to use the feature name correctly and fix the imports so the code runs.
Maintain the explicit `main` so that we can return an error using the `?` operator. Remove the empty `main` because its not needed anymore, it is a hang-over from Rust back in the day (before main was automatically added, IIUC).
ACKs for top commit:
apoelstra:
ACK 8bb0d3f667 successfully ran local tests
Tree-SHA512: 27f571ac3644417c06d0b4eb6fb122b39ac1068aefa4bcfc03f1febe2d031fb30616883c55c42c2ec80d419572fe7eba9bcc239e3c0e0e178ec7eaf8533b9efe
In b9643bf3e9 we introduced an incorrect
`cfg` attribute, that has just shown up, no clue why clippy only just
presented me with this error now. Anywho, the current code is buggy and
the rustdoc tests are never being run.
Fix `cfg` attribute to use the feature name correctly and fix the
imports so the code runs.
Maintain the explicit `main` so that we can return an error using the
`?` operator. Remove the empty `main` because its not needed anymore,
it is a hang-over from Rust back in the day (before main was
automatically added, IIUC).
8b9d657667 hashes: Remove unused file (Tobin C. Harding)
Pull request description:
Recently we moved the tests from `impls` to `tests/io.rs` but forgot to delete the original file.
Remove the already unused `impls.rs` file.
ACKs for top commit:
apoelstra:
ACK 8b9d657667 successfully ran local tests
Tree-SHA512: f701825239aff18faa0cd7adf69e1a8830dfb532df9d52c6d545e6352054ae6cb8c625c9fc05b7db374822d3478acf16858bc38ffbecf45575f555eb80f010e1
58704c2eff Remove schemars all together (Tobin C. Harding)
Pull request description:
We introduced schemars as a personal favor to a user, and it broke our CI repeatedly but eventually it seemed like it was stable (mainly, our MSRV caught up with its MSRV) so we just let it slide. In the end having schemars on hashes but nowhere else in the rust-bitcoin ecosystem did not prove that useful.
Remove schemars all together.
Fix: #3393
ACKs for top commit:
apoelstra:
ACK 58704c2eff successfully ran local tests
Tree-SHA512: 11c136797f28903c7d6b5199ad55d86bc4bc29ee8dd6f0d575e029f4dbebebabed57ebce6cf773b286297ea84f18d0b6cc58e150299e99457e048226478b49cc
cbfddb0394 hashes: Rename length field and use u64 (Tobin C. Harding)
Pull request description:
The hash engine types have a `length` field that is used to cache the number of bytes hashed so far, as such it is an arbitrary number and could use a `u64` instead of `usize`.
While we are at it rename `length` to `bytes_hashed` to remove any ambiguity of what this field is. Note this field is private, we already have the public getter `n_bytes_hashes` to get the value.
Introduce a private function `incomplete_block_size`, the purpose of this function is to put all the casts in one place so they can be well documented and easily understood.
Fix: #3016
ACKs for top commit:
apoelstra:
ACK cbfddb0394 successfully ran local tests
Tree-SHA512: a9d932938afcbd6dfb9db471a02fa7e3fff8f0659906627001ad241390b9af57088fd34afeae551c70c2c49783e6296f110b57ff9de6fed2609f4648ec8fd934
We introduced schemars as a personal favor to a user, and it broke our
CI repeatedly but eventually it seemed like it was stable (mainly, our
MSRV caught up with its MSRV) so we just let it slide. In the end having
schemars on hashes but nowhere else in the rust-bitcoin ecosystem did
not prove that useful.
Remove schemars all together.
Fix: #3393
f6abdcc001 Allow unused in `macros.rs` docs (Jamil Lambert, PhD)
fd89ddf401 Remove or fix unused variables and methods in docs (Jamil Lambert, PhD)
ff6b1d4f19 Remove unused variables and methods from docs (Jamil Lambert, PhD)
e58cda6f92 Remove `unused_imports` in docs (Jamil Lambert, PhD)
Pull request description:
As mentioned in #3362 examples in documentation are not linted in the same way as other code, but should still contain correctly written code.
#![doc(test(attr(warn(unused))))] has been added to all lib.rs files
In the docs throughout all crates:
- Unused imports have been removed.
- Unused variables, structs and enums have been used e.g. with an `assert_eq!` or prefixed with `_`
- Unused methods have been called in the example code.
ACKs for top commit:
tcharding:
ACK f6abdcc001
apoelstra:
ACK f6abdcc001 successfully ran local tests
Tree-SHA512: c3de1775ecde6971056e9fed2c9fa1621785787a6a6ccbf3a6dbd11e18d42d4956949f3f8adfc75d94fd25db998b04adb1c346cc2c2ba47f4dc37402e1388277
Examples in documentation are not linted in the same way as other code,
but should still contain correctly written code.
Throughout all of the crates except internals (another commit) unused
variables have been prefixed with `_`, unused imports have been removed,
and a warn attribute added to all of the `lib.rs` files.
25d906d936 Use UFCS in macros (Liam Aharon)
Pull request description:
Closes#3304
ACKs for top commit:
apoelstra:
ACK 25d906d936 successfully ran local tests
tcharding:
ACK 25d906d936
Tree-SHA512: a02a8507bcec73dab1ec8d49e45eab327d989c276d931520e0fff312faf7d3165292c300fb9414314f9b94b1255ee49a0fb64303df6d971c9089226b6873c36a
Tobin C. Harding
Jamil Lambert, PhD
merge-script
calciumbe
Leo Nash
Fmt Bot
Shing Him Ng
Liam Aharon