Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
tanriol
@tanriol:matrix.org
[m]
Ничего радикально ужасного там вроде бы не видно. Возможно, стоит спрашивать об их пожеланиях к коду их, а не нас...
Anton Sinitsyn
@sinitcin

for (_input_idx, input) in inputs.iter().enumerate() - вот и зачем делать enumerate только ради того, чтобы проигнорировать индекс?
outpoint.output_index() as usize - а это каст с потерей информации или без? Не зная оригинальный тип, не скажешь. Многие за это не любят as-касты.

Согласен косяк

По поводу каста, output_index() возвращает u32, аргумент был такой, мол, после сериализации данных все типы должны иметь фиксированный размер, тогда как usize может размер менять. Иначе ноды собранные для x86 и x64 могут не прочитать данные друг друга по сети
Anton Sinitsyn
@sinitcin

match с одним "рабочим" вариантом обычно принято писать как if let, если задача не в том, чтобы гарантированно отрабатывать новые варианты в случае их добавления. Отдельный вопрос, зачем в хвостовой позиции continue вместо {}

За if let ругаются у нас, типа не очевидно как раз будут ли новые элементы и не очевидно обработаны ли все элементы.

По поводу, continueтоже косяк

tanriol
@tanriol:matrix.org
[m]
Тогда я бы это писал как let index: usize = outpoint.output_index().try_into().expect("16-bit systems not supported"); (а скорее сделал бы, чтобы метод output_index делал это внутри себя и возвращал уже usize)...
Anton Sinitsyn
@sinitcin

Ничего радикально ужасного там вроде бы не видно. Возможно, стоит спрашивать об их пожеланиях к коду их, а не нас...

Ну собственно с этим я и пришёл сюда, что пожелания robust, secure, high quality)) Ну ладно, буду узнавать что конкретно не так, а не эти абстрактные термины

Спасибо
tanriol
@tanriol:matrix.org
[m]
Ну у них могут быть свои предпочтения :-) плюс я не смотрел на структуру кода "сверху", в смысле разбиение по функциям - возможно, там тоже что-то есть проблемное.
tanriol
@tanriol:matrix.org
[m]
А возможно, сказывается уже мой недостаток опыта и я не вижу крупные проблемы - тут уже упс, опыт у меня ограниченный и специфический.
Anton Sinitsyn
@sinitcin
Аналогично, да и с разработкой, то пусто, то густо. Несколько лет вообще только тим лидерством занимался
sobaka_v_kepke
@chabapok_twitter
@sinitcin Сразу хочу сказать, что когда пристебіваюсь я - то может и зря. Навскидку что бросилось в глаза. Строка 231 break Ok(block_index_walk), и аналогичные - это на самом деле не брик а ретурн. Такой стиль сбивает с толку. Я бы писал break только если там что-то типа let v = loop{ .... break result....}. А если это возврат - то это ретурн! Форматирование - ну на любителя. Во-первых, я бы разделял функции пустой строчкой. Между строкой 59 и 60 (и по всему коду аналогично) я бы вставил пустую. Кроме этого, я предпочитаю, чтобы аргументы функций были в строчку, а не в столбик. Но когда их много и функцию не разобьешь - то приходится и в столбик. Но я стараюсь не делать так. Но честно говоря, о стиле просто договариваются заранее, так что это ж не предмет, к которому пристебываться надо. Между сторокой с импл и функцией я бы тоже вставил пустую строчку(например - между 93 и 94). Всякие log::trace! я б заменил на trace!. Но это все неособо принципиальные замечания. Может, у них были претензии именно к архитектуре? Что еще? Например, в 156 строке использование Arc предполагает, что это все многопоточный код. Если код однопоточный, то там Rc уместней (но все зависит от общей архитектуры, о которой я не знаю ничего). И кстати, говоря. В строках 754-760 дергается запись в базу данных. Там идет add_to_block_index, set_block_index, add_block. Я ничего не знаю о базе и может я не прав - но если это все выполняется в несколько потоков (на что намекает упомянутый выше Arc), то эти 3 операции будут не атомарны? И если это действительно так - то вот это и было реальной причиной недовльства! А еще db_tx имеет тип BlockchainStorageRead - хотя там запись в строках 754-760. Может его надо просто обозвать BlockchainStorage, если там и запись тоже.
А подробней - это уже надо вникать.
sobaka_v_kepke
@chabapok_twitter
Я сделал парсер xml который умеет "вклиниваться в середину парсинга", и парсить енумы разделенные запятой. При этом сам парсер незнает, какой тип у этих енумов. Делюсь, принимаю критику https://www.rustexplorer.com/b/lq083u
плейграунд не подошел потому, что он не умеет xml
tanriol
@tanriol:matrix.org
[m]
Симпатичный адаптер. Что до "плейграунд не подошёл" - обрати внимание, он с тем же успехом работает для JSON, ничего XML-специфичного в нём нет.
sobaka_v_kepke
@chabapok_twitter
да. Но вобщем по-утру я увидел, что его надо еще причесать, например пустые строки в пустой массив он не конвертит. Но то уже - дело техники.
tanriol
@tanriol:matrix.org
[m]
Строго говоря, можно обойтись просто struct CommaSeparated<T>(Phantom<T>), этот тип от D не зависит.
И, скорее всего, PhantomData<T> слишком жёстко, но это некритично, особенно для такого специализированного типа.
sobaka_v_kepke
@chabapok_twitter
а что значит слишком жестко? Что можно "помягче" туда сунуть?
tanriol
@tanriol:matrix.org
[m]
Я бы там попробовал что-нибудь в стиле PhantomData<fn() -> T>. Дело в том, что конкретный параметр в PhantomData (как и типы конкретных полей) имеет сразу несколько неочевидных побочных эффектов, хотя в данной ситуации - когда этот тип не предназначен для пользователя и не хранится долгосрочно - большая часть их неактуальна.
sobaka_v_kepke
@chabapok_twitter
мне пока неочевидно, в чем разница. Я с этой фантомдатой пока полностью не разобрался еще
tanriol
@tanriol:matrix.org
[m]
PhantomData<T> обозначает "это поле пустое, но считай, что там находится значение типа T". Это влияет на несколько разных вещей. Например, если T: !Sync, то и вся структура будет !Sync, аналогично для Send. Если для T реализован Drop, это явно учитывается в drop check. Если T имеет внутри себя какое-то время жизни, то твоя структура будет по нему ковариантна (т.е. структуру с большим ВЖ можно использовать в качестве структуры с меньшим ВЖ). Если я не путаю, есть ещё семейство трейтов UnwindSafe с аналогичными свойствами.
sobaka_v_kepke
@chabapok_twitter
примерно ясно
tanriol
@tanriol:matrix.org
[m]
В общем, имеет смысл соответствовать сути происходящего. Не хранит, не ссылается, но может по заказу произвести T? Больше всего это похоже на fn() -> T.
sobaka_v_kepke
@chabapok_twitter
спасибо. Ясненько.
sobaka_v_kepke
@chabapok_twitter
в этой истории вопрос со строчкой let r: T = T::deserialize::<serde::de::value::StrDeserializer<'_, E>>(v.into_deserializer())?;
дело в том, что serde::de::value::StrDeserializer - недокументированная структура
но без явного ее указазания не компилится - неможет понять, какой класс у ошибки
tanriol
@tanriol:matrix.org
[m]
Странно, у меня let r: T = T::deserialize(v.into_deserializer())?; вроде бы работает
sobaka_v_kepke
@chabapok_twitter
хм.
Вчера не работало
tanriol
@tanriol:matrix.org
[m]
#[inline(always)] и не должен работать - он влияет на оптимизации, но не на семантику.
И нет, в таком стиле поумнее нет.
Chikirev Sirguy
@cheblin
поправка небольшая. этот пример я сильно переупростил, так что потерялся смысл
сейчас попробую более реалистичный пример сделать
Nickolay Novikov
@nnovikov:matrix.org
[m]
а вот подскажите, пытаюсь тут использовать rust-s3
29  |     let bucket = Bucket::new(bucket_name, region, credentials)?;
    |                  -----------                      ^^^^^^^^^^^ expected struct `s3::awscreds::Credentials`, found struct `awscreds::Credentials`
    |                  |
    |                  arguments to this function are incorrect
    |
    = note: perhaps two different versions of crate `awscreds` are being used?
это по сути одна и та же либа, но конпелятор этого не понимает, так как по разным путям прописан путь
как это правильно исправить?
tanriol
@tanriol:matrix.org
[m]
А какой версии rust-s3 и aws-creds вписаны в Cargo.toml?
Nickolay Novikov
@nnovikov:matrix.org
[m]
rust-s3 = { version = "0.32.3", features = ["sync"], default-features = false }
aws-creds = { version = "0.31.0", features = ["http-credentials"], default-features = false }
последнее, cargo add и вперед
tanriol
@tanriol:matrix.org
[m]
Поменяй aws-creds на 0.30.0 - rust-s3 ещё не обновлён на ветку 0.31
(ну или можно его вообще убрать и пользоваться реэкспортом s3::awscreds)
Nickolay Novikov
@nnovikov:matrix.org
[m]
use s3::creds::Credentials;
let credentials = Credentials::default()?; 

23 |     let credentials = Credentials::default()?;
   |                                    ^^^^^^^ function or associated item not found in `s3::awscreds::Credentials`
тогда как-то так выходит
tanriol
@tanriol:matrix.org
[m]
А, да, полностью убрать не получится - флаг http-credentials прокинуть не получится.
Nickolay Novikov
@nnovikov:matrix.org
[m]
а как бы так сделать что бы оно само правильную версию использовало?
tanriol
@tanriol:matrix.org
[m]
При имеющихся условиях, насколько я знаю, никак.
Nickolay Novikov
@nnovikov:matrix.org
[m]
понятно, будем страдать
всем спасибо
Nickolay Novikov
@nnovikov:matrix.org
[m]
[del]