refactor(sia): Sia http client refactor and #2108
Conversation
| macro_rules! define_byte_array_const { | ||
| ($name:ident, $size:expr, $value:expr) => { | ||
| pub const $name: [u8; $size] = { | ||
| let mut arr = [0u8; $size]; | ||
| let bytes = $value.as_bytes(); | ||
| let mut i = 0; | ||
| while i < bytes.len() && i < $size { | ||
| arr[i] = bytes[i]; | ||
| i += 1; | ||
| } | ||
| arr | ||
| }; | ||
| }; | ||
| } | ||
|
|
||
| define_byte_array_const!(ED25519, 16, "ed25519"); | ||
| define_byte_array_const!(SIACOIN_OUTPUT, 16, "siacoin output"); | ||
| define_byte_array_const!(SIAFUND_OUTPUT, 16, "siafund output"); | ||
| define_byte_array_const!(FILE_CONTRACT, 16, "file contract"); | ||
| define_byte_array_const!(STORAGE_PROOF, 16, "storage proof"); | ||
| define_byte_array_const!(FOUNDATION, 16, "foundation"); | ||
| define_byte_array_const!(ENTROPY, 16, "entropy"); |
There was a problem hiding this comment.
Wow!
That's very clever. Never knew runtime computed expressions can be assigned to constants.
| impl Encodable for Specifier { | ||
| fn encode(&self, encoder: &mut Encoder) { encoder.write_slice(self.identifier.as_bytes()); } | ||
| } | ||
|
|
There was a problem hiding this comment.
This isn't a protocol level thing right? I see this will encode the bytes of strings defined above (ed25519, siacoin output, etc...).
If this is just used to distinguish between different specifier on our side then we could possibly replace these strings with numbers instead, and encode those.
There was a problem hiding this comment.
This is a direct port of their encoding. It's intentionally encoding ASCII.
mm2src/coins/sia/address.rs
Outdated
| @@ -21,6 +22,10 @@ impl Address { | |||
| } | |||
| } | |||
|
|
|||
| impl Encodable for Address { | |||
| fn encode(&self, encoder: &mut Encoder) { encoder.write_slice(&self.0 .0.as_ref()); } | |||
There was a problem hiding this comment.
nit:
| fn encode(&self, encoder: &mut Encoder) { encoder.write_slice(&self.0 .0.as_ref()); } | |
| fn encode(&self, encoder: &mut Encoder) { self.0.encode(encoder) } |
| type Response = AddressBalanceResponse; | ||
|
|
||
| fn to_http_request(&self, base_url: &Url) -> Result<Request, SiaApiClientError> { | ||
| // TODO use .join method of Url to prevent any possibility of path traversal |
There was a problem hiding this comment.
I think if self.address is a valid address (h256), there won't be any possibility for path traversal.
| impl Encodable for SiacoinOutput { | ||
| fn encode(&self, encoder: &mut Encoder) { | ||
| self.value.encode(encoder); | ||
| self.address.encode(encoder); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think it would be less error-prone to not implement Encodable for the un-versioned types and only use their version-ers to encode them.
| } | ||
|
|
||
| // SiacoinOutput remains the same data structure between V1 and V2 however the encoding changes | ||
| pub enum SiacoinOutputVersion { |
There was a problem hiding this comment.
If the version-ers are only used for encoding, they can hold a reference to avoid unnecessary copying/cloning.
mm2src/coins/sia/spend_policy.rs
Outdated
| } | ||
|
|
||
| impl Encodable for PublicKey { | ||
| fn encode(&self, encoder: &mut Encoder) { encoder.write_len_prefixed_bytes(&self.to_bytes()); } |
There was a problem hiding this comment.
It feels like pubkeys shouldn't be len prefixed, since the length isn't variable.
Changelog
Add Docker Test Framework
Adapted the existed Docker test framework to allow running a self contained "regtest-like" blockchain network including instant block mining, a custom developer subsidy address and v1/v2 activation time frames.
See
sia_docker_tests.rs. Currently usesdocker.io/alrighttt/walletd-komodowhich utilizes my custom fork of walletd,https://github.com/Alrighttt/walletd/tree/Komodo-persistent-testnet.Refactor
http_clientmoedule and Addhttp_endpointsModuleRefactors the
SiaApiClientsignificantly for better developer ease of use. Introduces thedispatchermethod for SiaApiClient. This method is likely to be the main way a developer can interact with the Sia walletd API. See unit tests insia_docker_tests.rsfor example usage.Moves the individual endpoint implementations into a seperate
http_endpointsmodule.Add
specifierModuleAdds a small module to initialize the equivalent of Sia Go's "Identifier" encoding. These identifiers are used extensively in the Sia's transaction and block encoding.
See
specifier.rs.Add
transactionmoduleBegin work on transaction encoding and decoding. This module will ultimately implement both Sia v1 and v2 consensus. Adds many unit tests.
See
transaction.rs. See https://github.com/Alrighttt/core/blob/rust-port-unit-tests/types/rust_port_test.go .Add
EncodabletraitAdds the
Encodabletrait to allow better mimicking the code patterns of Sia's Go developers.See
encoding.rs. Seetransaction.rsunit tests for usage.Extend Api Client Error Handling
Refactors the
fetch_and_parsefunction of the API client to begin implementing more graceful handling of error messages orTodo
Remove or finalize developer comments
Add inline docs.rs documentation
Handle unexpected HTTP responses. Eg, HTTP 200 but unexpected data structure
Update Go port unit tests
Better organization of the Go/Rust equivalent unit tests. Ie,
rust_port_test.go