{"action":"opened","issue":{"url":"https://api.github.com/repos/status-im/nimbus/issues/72","repository_url":"https://api.github.com/repos/status-im/nimbus","labels_url":"https://api.github.com/repos/status-im/nimbus/issues/72/labels{/name}","comments_url":"https://api.github.com/repos/status-im/nimbus/issues/72/comments","events_url":"https://api.github.com/repos/status-im/nimbus/issues/72/events","html_url":"https://github.com/status-im/nimbus/issues/72","id":342248660,"node_id":"MDU6SXNzdWUzNDIyNDg2NjA=","number":72,"title":"Review implementation of SDIV","user":{"login":"mratsim","id":22738317,"node_id":"MDQ6VXNlcjIyNzM4MzE3","avatar_url":"https://avatars3.githubusercontent.com/u/22738317?v=4","gravatar_id":"","url":"https://api.github.com/users/mratsim","html_url":"https://github.com/mratsim","followers_url":"https://api.github.com/users/mratsim/followers","following_url":"https://api.github.com/users/mratsim/following{/other_user}","gists_url":"https://api.github.com/users/mratsim/gists{/gist_id}","starred_url":"https://api.github.com/users/mratsim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mratsim/subscriptions","organizations_url":"https://api.github.com/users/mratsim/orgs","repos_url":"https://api.github.com/users/mratsim/repos","events_url":"https://api.github.com/users/mratsim/events{/privacy}","received_events_url":"https://api.github.com/users/mratsim/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-07-18T09:31:34Z","updated_at":"2018-07-18T09:31:34Z","closed_at":null,"author_association":"MEMBER","body":"Following #71 by @yglukhov, implementation of SDIV follows closely Parity implementation:\r\n\r\n```Rust\r\n            instructions::SDIV => {\r\n                let (a, sign_a) = get_and_reset_sign(stack.pop_back());\r\n                let (b, sign_b) = get_and_reset_sign(stack.pop_back());\r\n\r\n                // -2^255\r\n                let min = (U256::one() << 255) - U256::one();\r\n                stack.push(if self.is_zero(&b) {\r\n                    U256::zero()\r\n                } else if a == min && b == !U256::zero() {\r\n                    min\r\n                } else {\r\n                    let c = a.overflowing_div(b).0;\r\n                    set_sign(c, sign_a ^ sign_b)\r\n                });\r\n            },\r\n```\r\n\r\nhttps://github.com/status-im/nimbus/blob/6c67115ef519a3467ba2aed973e881154d15b2f6/nimbus/vm/interpreter/opcodes_impl.nim#L45-L60\r\n\r\nHowever Parity seems to handle the case of `low(int256)` incorrectly:\r\n\r\n1. It first get and reset the sign of operand\r\n    low(Int256) = 0b1000_0000\r\n    taking the inverse is flipping bit + 1\r\n    * flipping: 0b0111_1111\r\n    * +1:  0b1000_0000\r\n    i.e. like 0, low(int) inversion in memory gives the same bit pattern (see Wikipedia [Two-complement](https://en.wikipedia.org/wiki/Two%27s_complement).\r\n\r\n2. Parity defines `min = (U256::one() << 255) - U256::one();` but it should be `min = (U256::one() << 255)` only.\r\n\r\nSee Nim test case:\r\n\r\n```Nim\r\nimport stint, strutils\r\n\r\necho \"1 shl 7: \" & (1'i8 shl 7).toBin(8)\r\n\r\nlet x = -128.int8\r\necho \"x: \" & x.toBin(8)\r\n\r\nlet y = (not cast[uint8](x)) + 1\r\necho \"y: \" & cast[int8](y).toBin(8)\r\n\r\n################################################################\r\necho \"\\n 1 shl 255\"\r\nlet a = one(UInt256) shl 255\r\necho a.toString(base = 2).align(256, padding = '0')\r\necho a.dumpHex\r\n\r\necho \"\\n 1 shl 255 - 1\"\r\nlet b = a - 1\r\necho b.toString(base = 2).align(256, padding = '0')\r\necho b.dumpHex\r\n\r\necho \"\\n high(Int256)\"\r\necho high(Int256).toString(base = 2).align(256, padding = '0')\r\necho high(Int256).dumpHex\r\n\r\necho \"\\n low(Int256)\"\r\n# echo low(Int256).toString(base = 2).align(256, padding = '0') # Throws underflow exception\r\necho low(Int256).dumpHex\r\n```\r\n\r\n```Nim\r\n1 shl 7: 10000000\r\nx: 10000000\r\ny: 10000000\r\n\r\n 1 shl 255\r\n1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000\r\n8000000000000000000000000000000000000000000000000000000000000000\r\n\r\n 1 shl 255 - 1\r\n0111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111\r\n7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\r\n\r\n high(Int256)\r\n0111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111\r\n7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\r\n\r\n low(Int256)\r\n8000000000000000000000000000000000000000000000000000000000000000\r\n```\r\n\r\nAt minimm tests in test_opcodes are needed to complement EVM tests."}}
{"actor":{"display_login":"mratsim"}}