Author Topic: Error Registering Premium Names  (Read 3983 times)

0 Members and 1 Guest are viewing this topic.

Offline pc

  • Hero Member
  • *****
  • Posts: 1530
    • View Profile
    • Bitcoin - Perspektive oder Risiko?
  • BitShares: cyrano
You were playing fire IMO. :P And also CNX. Maybe I'm too conservative.

Anyway it's good news so far. GUI can upgrade sooner to adapt new syntax.

I guess it's good news that the network layer dropped the "type" (array/object) and the "brackets" ( { } / [ ] ) , so both upgraded and non-upgraded witness can correctly decode the transactions when "extensions" field is empty. And maybe same behavior on data storage (database) layer? Haven't have so much time to read all the codes..

Well, I only checked the syntax using get_prototype_operation, fixed my voting script and ran it. Only when I saw the forum thread I started digging deeper. Maybe I should have been more careful.

What I'd really like to know is if the compatibility of the old and new binary representations was on purpose or by accident. @theoretical  @bytemaster ?
Bitcoin - Perspektive oder Risiko? ISBN 978-3-8442-6568-2 http://bitcoin.quisquis.de

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
@svk Don't deploy the new code before the hard fork date, otherwise it will perhaps cause network forking since it's not sure that all witnesses are updated before that date.

I think the change is not dangerous as long as you keep the extensions empty. (Actually I updated my votes yesterday using the new release, after discovering the bug...)
You were playing fire IMO. :P And also CNX. Maybe I'm too conservative.

Anyway it's good news so far. GUI can upgrade sooner to adapt new syntax.

I guess it's good news that the network layer dropped the "type" (array/object) and the "brackets" ( { } / [ ] ) , so both upgraded and non-upgraded witness can correctly decode the transactions when "extensions" field is empty. And maybe same behavior on data storage (database) layer? Haven't have so much time to read all the codes..
BitShares committee member: abit
BitShares witness: in.abit

Offline pc

  • Hero Member
  • *****
  • Posts: 1530
    • View Profile
    • Bitcoin - Perspektive oder Risiko?
  • BitShares: cyrano
The historical operations will definitely be validated while replaying. However there was no issue while replaying, so historical data can be converted to the new structure with new code with no problem. Looks like the new "from_variant_visitor" and "unpack_visitor" and related code did the trick: https://github.com/bitshares/bitshares-2/commit/a1e8fc07417407525482924df6b125d95c38a06f#diff-81ae2fb7b3dfa822471637c59a33c817R143.

So the real issue is parameters of API calls aren't converted in the same way.
@pc

//Update:
So in theoretical it's possible to convert an array-like string/variant_object to an extension<T> object, but the API server never tried to convert because of this judgement statement: https://github.com/cryptonomex/fc/blob/e5ca765f1508f2a05bb980231ee8e4b8985b51cf/src/variant.cpp#L573 ?

I think it is possible only in the special case where both are empty.
The old extensions_type is a flat_set, which is packed  as a size field followed by the elements. So an empty set is packed into a zero count.
The new graphene_extension_unpack_visitor packs the extension as a count of non-empty fields followed by (index,value) pairs. So an empty extensions field is packed into a zero count as well.
@theoretical please confirm.

But the variant stuff works differently. to_variant encodes a flat_set into an array and a struct into an object. from_variant checks these type, which then throws the exception.

@svk Don't deploy the new code before the hard fork date, otherwise it will perhaps cause network forking since it's not sure that all witnesses are updated before that date.

I think the change is not dangerous as long as you keep the extensions empty. (Actually I updated my votes yesterday using the new release, after discovering the bug...)
Bitcoin - Perspektive oder Risiko? ISBN 978-3-8442-6568-2 http://bitcoin.quisquis.de

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
The reason for this is an undocumented API change in the latest release. The "extensions" field on account create/update operations is now a struct not an array. Which is precisely the reason why I think one week is far from sufficient time to introduce a hard fork!

https://github.com/bitshares/bitshares-2/commit/a1e8fc07417407525482924df6b125d95c38a06f#diff-1a2d7c55b5a8f1e2bbb1c868896c94eeL117

Thanks for finding the cause, I'll make the changes to the GUI where needed.
@svk Don't deploy the new code before the hard fork date, otherwise it will perhaps cause network forking since it's not sure that all witnesses are updated before that date.
« Last Edit: February 20, 2016, 03:53:37 pm by abit »
BitShares committee member: abit
BitShares witness: in.abit

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
I'm curious why the modified operation structures are compatible with old transactions. @pc ?

Good question. I'd guess that so far the extensions haven't been used anywhere, and that the serialized representation of an empty set is the same as that of an empty struct? (Except in JSON, which is what's causing the problem here.)
The historical operations will definitely be validated while replaying. However there was no issue while replaying, so historical data can be converted to the new structure with new code with no problem. Looks like the new "from_variant_visitor" and "unpack_visitor" and related code did the trick: https://github.com/bitshares/bitshares-2/commit/a1e8fc07417407525482924df6b125d95c38a06f#diff-81ae2fb7b3dfa822471637c59a33c817R143.

So the real issue is parameters of API calls aren't converted in the same way.
@pc

//Update:
So in theoretical it's possible to convert an array-like string/variant_object to an extension<T> object, but the API server never tried to convert because of this judgement statement: https://github.com/cryptonomex/fc/blob/e5ca765f1508f2a05bb980231ee8e4b8985b51cf/src/variant.cpp#L573 ?
« Last Edit: February 20, 2016, 03:48:43 pm by abit »
BitShares committee member: abit
BitShares witness: in.abit

Offline pc

  • Hero Member
  • *****
  • Posts: 1530
    • View Profile
    • Bitcoin - Perspektive oder Risiko?
  • BitShares: cyrano
I'm curious why the modified operation structures are compatible with old transactions. @pc ?

Good question. I'd guess that so far the extensions haven't been used anywhere, and that the serialized representation of an empty set is the same as that of an empty struct? (Except in JSON, which is what's causing the problem here.)
Bitcoin - Perspektive oder Risiko? ISBN 978-3-8442-6568-2 http://bitcoin.quisquis.de

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
It's a sad stupid human error.  :(

I'm curious why the modified operation structures are compatible with old transactions. @pc ?

OP: by now, you can try current GUI (2.0.160208) with an older witness_node (2.0.160128).
« Last Edit: February 20, 2016, 11:42:57 am by abit »
BitShares committee member: abit
BitShares witness: in.abit


Offline svk

The reason for this is an undocumented API change in the latest release. The "extensions" field on account create/update operations is now a struct not an array. Which is precisely the reason why I think one week is far from sufficient time to introduce a hard fork!

https://github.com/bitshares/bitshares-2/commit/a1e8fc07417407525482924df6b125d95c38a06f#diff-1a2d7c55b5a8f1e2bbb1c868896c94eeL117

Thanks for finding the cause, I'll make the changes to the GUI where needed.
Worker: dev.bitsharesblocks

Offline pc

  • Hero Member
  • *****
  • Posts: 1530
    • View Profile
    • Bitcoin - Perspektive oder Risiko?
  • BitShares: cyrano
The reason for this is an undocumented API change in the latest release. The "extensions" field on account create/update operations is now a struct not an array. Which is precisely the reason why I think one week is far from sufficient time to introduce a hard fork!

https://github.com/bitshares/bitshares-2/commit/a1e8fc07417407525482924df6b125d95c38a06f#diff-1a2d7c55b5a8f1e2bbb1c868896c94eeL117
Bitcoin - Perspektive oder Risiko? ISBN 978-3-8442-6568-2 http://bitcoin.quisquis.de

Offline ByronP

  • Full Member
  • ***
  • Posts: 70
    • View Profile
When trying to register a premium name an unknown error message is shown and the console shows the following error:

Code: [Select]
!!! GrapheneApi error:  get_required_fees [Array[1], "1.3.0"]0: Array[1]0: Array[2]0: 51: Objectactive: Objectextensions: Array[0]fee: Objectname: "nameredacted"options: Objectowner: Objectreferrer: "1.2.00000"referrer_percent: 0registrar: "1.2.00000"__proto__: Objectlength: 2__proto__: Array[0]length: 1__proto__: Array[0]1: "1.3.0"length: 2__proto__: Array[0] Object {code: 1, message: "7 bad_cast_exception: Bad Cast↵Invalid cast from t…array_type"}↵    th_a  variant.cpp:575 get_object", data: Object}code: 1data: Objectmessage: "7 bad_cast_exception: Bad Cast↵Invalid cast from type 'array_type' to Object↵    {"type":"array_type"}↵    th_a  variant.cpp:575 get_object"__proto__: Object {"code":1,"message":"7 bad_cast_exception: Bad Cast\nInvalid cast from type 'array_type' to Object\n    {\"type\":\"array_type\"}\n    th_a  variant.cpp:575 get_object","data":{"code":7,"name":"bad_cast_exception","message":"Bad Cast","stack":[{"context":{"level":"error","file":"variant.cpp","line":575,"method":"get_object","hostname":"","thread_name":"th_a","timestamp":"2016-02-19T01:28:37"},"format":"Invalid cast from type '${type}' to Object","data":{"type":"array_type"}}]}}
app.js:102 ERROR AccountActions.createAccount Object {code: 1, message: "7 bad_cast_exception: Bad Cast↵Invalid cast from t…array_type"}↵    th_a  variant.cpp:575 get_object", data: Object}code: 1data: Objectcode: 7message: "Bad Cast"name: "bad_cast_exception"stack: Array[1]0: Objectcontext: Objectdata: Objecttype: "array_type"__proto__: Objectformat: "Invalid cast from type '${type}' to Object"__proto__: Objectlength: 1__proto__: Array[0]__proto__: Objectmessage: "7 bad_cast_exception: Bad Cast↵Invalid cast from type 'array_type' to Object↵    {"type":"array_type"}↵    th_a  variant.cpp:575 get_object"__proto__: Object__defineGetter__: __defineGetter__()__defineSetter__: __defineSetter__()__lookupGetter__: __lookupGetter__()__lookupSetter__: __lookupSetter__()constructor: Object()hasOwnProperty: hasOwnProperty()isPrototypeOf: isPrototypeOf()propertyIsEnumerable: propertyIsEnumerable()toLocaleString: toLocaleString()toString: toString()valueOf: valueOf()get __proto__: get __proto__()set __proto__: set __proto__()

I have redacted out some information such as name, referrer and registrar.

This error is thrown regardless of the name and the software is the latest light client (issue is confirmed on both windows and mac).