GolosChain / golos

Socio-economic mediablockchain
https://developers.golos.io
Other
59 stars 36 forks source link

Operations sequence #259

Closed asuleymanov closed 6 years ago

asuleymanov commented 6 years ago

Приветствую. Из-за изменения последовательности операций в массиве. Большинство сторонних библиотек на других ЯП при переходе на 17 версию не смогут работать. Для этого программистам библиотек необходимо будет переписывать их. А можно попросить сделать более правильно (на мой взгляд). Т.е. что я имею ввиду новые операции которые будут в 17 версии переместить в начало массива, а операции оставленные для 16 версии переместить в конец массива. Ведь сами операции и входные параметры не изменились. Но из-за сдвигания массива на 40 пунктов библиотеки и соответственно По ранее на них написанное перестанет работать. Вот как массив выглядит сейчас:

// _16 for 0.16.4
  vote_16: 0,
  comment_16: 1,
  transfer_16: 2,
  transfer_to_vesting_16: 3,
  withdraw_vesting_16: 4,
  limit_order_create_16: 5,
  limit_order_cancel_16: 6,
  feed_publish_16: 7,
  convert_16: 8,
  account_create_16: 9,
  account_update_16: 10,
  witness_update_16: 11,
  account_witness_vote_16: 12,
  account_witness_proxy_16: 13,
  pow_16: 14,
  //
  custom: 15,
  //
  report_over_production_16: 16,
  delete_comment_16: 17,
  //
  custom_json: 18,
  //
  comment_options_16: 19,
  set_withdraw_vesting_route_16: 20,
  limit_order_create2_16: 21,
  challenge_authority_16: 22,
  prove_authority_16: 23,
  request_account_recovery_16: 24,
  recover_account_16: 25,
  change_recovery_account_16: 26,
  escrow_transfer_16: 27,
  escrow_dispute_16: 28,
  escrow_release_16: 29,
  pow2_16: 30,
  escrow_approve_16: 31,
  transfer_to_savings_16: 32,
  transfer_from_savings_16: 33,
  cancel_transfer_from_savings_16: 34,
  //
  custom_binary: 35,
  //
  decline_voting_rights_16: 36,
  reset_account_16: 37,
  set_reset_account_16: 38,
  comment_benefactor_reward_16: 39,
  //
  vote: 40,
  comment: 41,
  transfer: 42,
  transfer_to_vesting: 43,
  withdraw_vesting: 44,
  limit_order_create: 45,
  limit_order_cancel: 46,
  feed_publish: 47,
  convert: 48,
  account_create: 49,
  account_update: 50,
  witness_update: 51,
  account_witness_vote: 52,
  account_witness_proxy: 53,
  pow: 54,
  report_over_production: 55,
  delete_comment: 56,
  comment_options: 57,
  set_withdraw_vesting_route: 58,
  limit_order_create2: 59,
  challenge_authority: 60,
  prove_authority: 61,
  request_account_recovery: 62,
  recover_account: 63,
  change_recovery_account: 64,
  escrow_transfer: 65,
  escrow_dispute: 66,
  escrow_release: 67,
  pow2: 68,
  escrow_approve: 69,
  transfer_to_savings: 70,
  transfer_from_savings: 71,
  cancel_transfer_from_savings: 72,
  decline_voting_rights: 73,
  reset_account: 74,
  set_reset_account: 75,
  comment_benefactor_reward: 76,
  delegate_vesting_shares: 77,
  account_create_with_delegation: 78,
  comment_payout_extension: 79,
  asset_create: 80,
  asset_update: 81,
  asset_update_bitasset: 82,
  asset_update_feed_producers: 83,
  asset_issue: 84,
  asset_reserve: 85,
  asset_fund_fee_pool: 86,
  asset_settle: 87,
  asset_force_settle: 88,
  asset_global_settle: 89,
  asset_publish_feed: 90,
  asset_claim_fees: 91,
  call_order_update: 92,
  account_whitelist: 93,
  override_transfer: 94,
  proposal_create: 95,
  proposal_update: 96,
  proposal_delete: 97,
  bid_collateral: 98,
bitphage commented 6 years ago

Yeah this change breaks compatibility with all libraries. Tested with python golos libs.

nemothenoone commented 6 years ago

Operations order is a backward compatibility requirement. All the operations signed before hardfork17 in some cases are getting identified by a number, not a name. So the order is a crucial stuff. This restriction is going to be removed in a softfork version following.

golos-js is already getting updated, golos-pyhton will be updated until the hardfork comes.

asuleymanov commented 6 years ago

Я не говорю про библиотеки на ЯП js и python. Или другие языки программирования Вас мало интересуют? Т.е. Вам как я понимаю все равно что у сторонних разработчиков приложения работать не будут?

nemothenoone commented 6 years ago

@asuleymanov We have started notifying all the community API libraries developers. That is why we have some time after public testing beginning - to help community developers introduce protocol versioning for API libraries.

asuleymanov commented 6 years ago

Так ладно понятно. Итого получается что это так и останется. Ладно потерпим не беда. Вы упоминали что это будет изменено после softfork. Как я понимаю это приведет к тому что все вернется обратно после перехода на ХФ? Если так то в какие сроки?

И уж если говорите что оповещали то можно посмотреть где это было сделано?

nemothenoone commented 6 years ago

@asuleymanov Protocol versioning is not going to be reverted because of protocol structure changes mechanism existence requirements (long asset names or more votable chain params for example). It will be widely used in future. But don't be scared. There are no any backward compatibility obligations for API libraries. It just needs to be changed once and done.

We've notified some of community library developers in personal, but not in public, because of a reference API libraries have not been fully updated yet and we did not finished updating our wiki and doxygen-generated documentation. You were in notification queue after Steepshot guys :) In case you asked in person, I think, the time has come.

Just to solve the trouble for now. Changes are all about asset precision and ticker name serialisation. Version 16 had asset serialisation like 1000 | \x03 | GOLOS <=> amount | precision_decimals | ticker_name <=> 8 bytes | 1 byte | 6 bytes. Now it is 1000 | GOLOS | \x03 <=> amount | ticker_name | decimals <=> 8 bytes | 16 bytes | 1 byte.

So all the operations contained asset, price, price_feed, chain_properties structures needs to be updated.

I think @b1acksun could help you deal with it.

asuleymanov commented 6 years ago

Спасибо большое за разъяснения.

nemothenoone commented 6 years ago

@asuleymanov I would really appreciate if you report the result of changes in golos-go. I mean, if it worked or not.

On 13 Nov 2017, at 17:32, Aleksey Suleymanov notifications@github.com wrote:

Спасибо большое за разъяснения.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

asuleymanov commented 6 years ago

Т.е. еще разок для закрепления у меня материала. Массив операций такой и будет, и если и будет изменяться то только добавляя новые операции?

За разъяснения с значением amount огромное спасибо. Так как этот вопрос стоял в списке, но до него я просто не дошел. Как только реализую тестовую вариацию массива обязательно переделаю и проверю работоспособность. Обязательно по результатам отпишусь тут.

asuleymanov commented 6 years ago

Приветствую. Я переделал библиотеку под 17 версию и большинство функций работает нормально.

Предложенный @Nemo1369 вариант был опробован и работает на отлично. Ну по крайней мере со стандартными токенами проблем не возникло. Буду тестировать и разрабатывать дальше. Кому интересно код можно посмотреть тут

И да @Nemo1369 большая просьба посмотреть issues/272.

nemothenoone commented 6 years ago

Thanks for the report @asuleymanov.

Talking about feature changes. Operations variant is only going to be extended until got replaced. The only important changes for API libraries developers will be described in doxygen-generated documentation.

t3ran13 commented 6 years ago

я чет не понял, у нас порядок операций изменится?

nemothenoone commented 6 years ago

@t3ran13 Operations set was extended for versioned operations with fundamental structure changes.

t3ran13 commented 6 years ago

но почему, например, операция vote теперь имеет порядок 40 вместо изначального 0? это приведет к тому, что все старые подписи операций станут недействительными.

nemothenoone commented 6 years ago

@t3ran13 Of course it is not, because stuff I've designed is about versioned operations. It means, daemon will be backward compatible with operations for versions 0-16, but it will not accept operations for version 17 just after hardfork time has come. Daemon with production network data is deployed at 46.101.132.158:2001 and 46.101.132.158:8090.

t3ran13 commented 6 years ago

Есть патерн ивент сорсинг, который запрещает такое делать. Может появиться vote_2.0 ,например, но старый останется как есть.

Зачем , в данном случае, наращивать энтропию в архитектуре приложения?

Как вы прикажете действовать тем кто будет сканировать блоки напрямую? До блока Х делайте так, после блока Х делацте сяк? При этом имя операции остается одним и тем же

У меня мозг отказывается понимать архитектуру такого приложения.

Я не понимаю почему нельзя это обойти по человечески? Зачем создавать костыль и инструменты для работы с кастылем? Почему нельзя ввести операции версии 2.0?

nemothenoone commented 6 years ago

@t3ran13 It was implemented exactly as you suppose. Internal implementation is like account_create_operation<0, 16, 0> for the versions 0-16 and account_create_operation<0, 17, 0> for the version 17, so operations proceeded are immutable. It satisfies the "Event Sourcing" pattern perfectly well (in fact it barely satisfies it).

Talking about JSON-RPC protocol - implementing account_create_operation_2 or account_create_operation_228 for each new protocol version makes it huge and messy. Actually, it was a tradeoff in favor of frontend developers to make the current communication protocol as tiny as we can. Full blockchain analysis would really require to implement multiple protocol versions support - just like golos-js does.

I think I just need to clean up the doxygen documentation to include the whole protocol description.

t3ran13 commented 6 years ago

т.е. данные в самой цепочке будут храниться корректно? а нечеловеческий вид лишь для rpc? при этом это вызано требованием сократить на 2 символа имя и сэкономить трафик? а если через пол года еще операции - добавяться/изменяться, то у нас получится еще одна полная фигня?

не, давайте по человечески все делать, и чб в rpc было согласно "Event Sourcing" - vote & vote_17 (или vote_2.0), но текущая реализация... как потом разрабом с этим апи работать? ты лично будешь каждому рассказывать как все реализовано и работает?

@all кто-то может прочекать код чб блоки не пострадали?

t3ran13 commented 6 years ago

детали все тут https://golos.io/ru--khardfork/@t3ran13/vazhno-vozmozhnye-kosyaki-v-dannykh-cepochki-iz-za-khf-vazhno-mnenie-razrabov#@nemo1369/re-t3ran13-re-nemo1369-re-t3ran13-vazhno-vozmozhnye-kosyaki-v-dannykh-cepochki-iz-za-khf-vazhno-mnenie-razrabov-20171209t194255420z

в общем - изменения для апи только, но для апи это хоть и криво, но не критично)

bitphage commented 6 years ago

Как вариант, можно сделать версионирование апи. Т.е. в каждом запросе/ответе слать атрибут вида "api_version: 1". Имена операций на клиенте будут одинаковыми, но для разных версий API соответственно будет меняться соответствие имя-номер.

t3ran13 commented 6 years ago

это тоже немного усложняет обработку, немного проще когда каждое поле имеет свою версию, если ответы оборачивать в объекты, это не проблема) хотя и ваш вариант по своему хорош. просто что делать когда ответ смешанный? тогда он не годится)

я просто понял, что сделано "как сделано" и никто переделывать сейчас желанием не горит