Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Base58 issues (2x) #1239

Merged
merged 7 commits into from
Nov 25, 2019
Merged

Conversation

shargon
Copy link
Member

@shargon shargon commented Nov 13, 2019

Port of #1224

@shargon shargon added the bug Used to tag confirmed bugs label Nov 13, 2019
Copy link
Contributor

@lock9 lock9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it generating address and all addresses continued the same after this change. Shouldn't it have broken the address conversion? I don't really know if I'm in favor of these changes for neo 2. First I want to understand why it did not break the code 😂

@shargon
Copy link
Member Author

shargon commented Nov 14, 2019

Because I think that only affect empty buffers

@superboyiii
Copy link
Member

@shargon Looks good, let me do some tests quickly.

@superboyiii
Copy link
Member

superboyiii commented Nov 15, 2019

@shargon I tried your version, it does fix this issue.
image
image
And I applied it on neo-cli, looks no influence on normal privkey<-->Address encoding and decoding.
image

image

Nice catch!

superboyiii
superboyiii previously approved these changes Nov 15, 2019
@shargon shargon requested a review from lock9 November 16, 2019 09:42
@shargon
Copy link
Member Author

shargon commented Nov 19, 2019

Changes was copied from #1224

@shargon
Copy link
Member Author

shargon commented Nov 19, 2019

@superboyiii Could your team ensure that it is a backward compatible? it's possible to be used in a SC.

https://github.com/neo-project/neo-devpack-dotnet/blob/34ade12e470894dcc3a7059630c5514f3299b95b/Neo.SmartContract.Framework/Helper.cs#L230

@superboyiii
Copy link
Member

@shargon Sure, let me check.

@superboyiii
Copy link
Member

superboyiii commented Nov 21, 2019

@shargon
I'm using the latest neo, neo-cli, dotnet-devpack, all on master-2.x.
I applied it on private net, tested import&export key, generate address, compared with 2.10.3 released version, deployed nep-5 contract and invoke transaction history, decoding scripthash inside to address. These are all OK but I found an issue that when I invoked my nep5 contract for making transaction, it breaked at here.
image
Here's the applicationlog for the successed and failed.
image

{
    "jsonrpc": "2.0",
    "id": 1,
    "result": {
        "txid": "0x2054c78fd3cd6da675cb63124686e46f3387449e542d9c7bc6013815d8f36963",
        "executions": [
            {
                "trigger": "Application",
                "contract": "0xc8b01b9a3db1bbf61b596c3384ffa592204553b8",
                "vmstate": "HALT",
                "gas_consumed": "3.068",
                "stack": [],
                "notifications": [
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "0000c16ff28623"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "66726f6d2062616c616e636520726563616c63756c617465207375636365737366756c6c79"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "726563616c63756c617465207375636365737366756c6c79"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "7472616e73666572"
                                },
                                {
                                    "type": "ByteArray",
                                    "value": "f07d73c2df532f7e31ad268dfe52e3c46d880319"
                                },
                                {
                                    "type": "ByteArray",
                                    "value": "0f37ea6a58060fecff83ba859182e70d203b34af"
                                },
                                {
                                    "type": "ByteArray",
                                    "value": "002a16edea00"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "66726f6d"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "f07d73c2df532f7e31ad268dfe52e3c46d880319"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "746f3a"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "0f37ea6a58060fecff83ba859182e70d203b34af"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "616d6f756e743a"
                                }
                            ]
                        }
                    },
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "002a16edea00"
                                }
                            ]
                        }
                    }
                ]
            }
        ]
    }
}
{
    "jsonrpc": "2.0",
    "id": 1,
    "result": {
        "txid": "0x1506b2a4da1dcaabcb6951ea1ee0b775863cd65bfcbfa2494efa734f17d9860f",
        "executions": [
            {
                "trigger": "Application",
                "contract": "0x84751f7016001d3c0a36b89a487d3f2d222a7f1c",
                "vmstate": "HALT",
                "gas_consumed": "0.766",
                "stack": [],
                "notifications": [
                    {
                        "contract": "0x74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                        "state": {
                            "type": "Array",
                            "value": [
                                {
                                    "type": "ByteArray",
                                    "value": "00a3181add65"
                                }
                            ]
                        }
                    }
                ]
            }
        ]
    }
}

Look, the fromAmount showed here is:
image
But actually my balance is:
image
The situation is that when I first deploy asset, I tried 3 times for making transactions by increasing transferred value. It was success. But when I tried decreaseing the value, my Runtime Notify showed that the fromAmount check was failed. But I do have enough asset.
image
Here's the transLog of my address.

{
    "jsonrpc": "2.0",
    "id": 1,
    "result": {
        "sent": [
            {
                "timestamp": 1574232287,
                "asset_hash": "9a61a7455a17d9aa1a4293c2537d3d768cdb184a",
                "transfer_address": "AHALmTxsMENorHVJ1VppJtSuVzpKXjtwQd",
                "amount": "1000000000000",
                "block_index": 816,
                "transfer_notify_index": 0,
                "tx_hash": "1a65afb1d8efc61bc5388726d96596a0d6e522d6d2487734dad4dcb4b1ea8127"
            },
            {
                "timestamp": 1574240208,
                "asset_hash": "8ce37a7f9f80d90c3084dcf52d0ecd3744643bb5",
                "transfer_address": "AHALmTxsMENorHVJ1VppJtSuVzpKXjtwQd",
                "amount": "93189000000000",
                "block_index": 2371,
                "transfer_notify_index": 0,
                "tx_hash": "463326668f9ac69349fc152ad012b04eda0d0da66017413dda41e4ea45459dde"
            },
            {
                "timestamp": 1574242869,
                "asset_hash": "cca3ce6036c74ceecb0e28d52c09c96e1f75670f",
                "transfer_address": "AHALmTxsMENorHVJ1VppJtSuVzpKXjtwQd",
                "amount": "131232100000000",
                "block_index": 2889,
                "transfer_notify_index": 0,
                "tx_hash": "90daf76541e7f7eed86623db1ba5d48ef88a3fa41fbb314c4825fa09b77927d7"
            },
            {
                "timestamp": 1574242874,
                "asset_hash": "cca3ce6036c74ceecb0e28d52c09c96e1f75670f",
                "transfer_address": "AHALmTxsMENorHVJ1VppJtSuVzpKXjtwQd",
                "amount": "434332100000000",
                "block_index": 2890,
                "transfer_notify_index": 0,
                "tx_hash": "545cd8a28e37125598c4db8a8f06ffb783f34f6685de0c54af3901bcfe91d46a"
            },
            {
                "timestamp": 1574243938,
                "asset_hash": "74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                "transfer_address": "AHALmTxsMENorHVJ1VppJtSuVzpKXjtwQd",
                "amount": "1009000000000",
                "block_index": 3100,
                "transfer_notify_index": 0,
                "tx_hash": "2054c78fd3cd6da675cb63124686e46f3387449e542d9c7bc6013815d8f36963"
            },
            {
                "timestamp": 1574243944,
                "asset_hash": "74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                "transfer_address": "AHALmTxsMENorHVJ1VppJtSuVzpKXjtwQd",
                "amount": "10090100000000",
                "block_index": 3101,
                "transfer_notify_index": 0,
                "tx_hash": "aafeaccf8961ee276dc40b3b130e0761ce07f2cd3acddccb5b7e5b003e3a520e"
            },
            {
                "timestamp": 1574243949,
                "asset_hash": "74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                "transfer_address": "AHALmTxsMENorHVJ1VppJtSuVzpKXjtwQd",
                "amount": "100901200000000",
                "block_index": 3102,
                "transfer_notify_index": 0,
                "tx_hash": "544bf1fd2264efb6a976d0ce451051ce6429223b2f87e741e91dca89b44792c8"
            }
        ],
        "received": [
            {
                "timestamp": 1574231867,
                "asset_hash": "9a61a7455a17d9aa1a4293c2537d3d768cdb184a",
                "transfer_address": "AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM",
                "amount": "10000000000000000",
                "block_index": 734,
                "transfer_notify_index": 0,
                "tx_hash": "cea8197014eb5fd4c738c7d065f5a83d124bb7c831bb41971f5cbbf022b4c524"
            },
            {
                "timestamp": 1574240173,
                "asset_hash": "8ce37a7f9f80d90c3084dcf52d0ecd3744643bb5",
                "transfer_address": "AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM",
                "amount": "10000000000000000",
                "block_index": 2364,
                "transfer_notify_index": 0,
                "tx_hash": "6b65c09a91998f2991df6b81e3fffe53c687b7809c75724d452a405e07f2388f"
            },
            {
                "timestamp": 1574242787,
                "asset_hash": "cca3ce6036c74ceecb0e28d52c09c96e1f75670f",
                "transfer_address": "AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM",
                "amount": "10000000000000000",
                "block_index": 2873,
                "transfer_notify_index": 0,
                "tx_hash": "29df8d3cabe8eed616d06fa094c12adb82ba83159c726b810916e9652968d72a"
            },
            {
                "timestamp": 1574243898,
                "asset_hash": "74c7ed614ae1d1a5ad6d704d37416072472bdfdf",
                "transfer_address": "AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM",
                "amount": "10000000000000000",
                "block_index": 3092,
                "transfer_notify_index": 0,
                "tx_hash": "f3b94ded7769547aed09ebc53ac43cc8f03b2d3e65c56848ee458efd58a69ca2"
            }
        ],
        "address": "AdhU77G7mbhnYgTe12ZAwFTomoP2MhsTbi"
    }
}

Is it possbile that there's incompatible between BigInteger and asset.Get(from).AsBigInteger() here? Here's my contract code.

using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Services.Neo;
using Neo.SmartContract.Framework.Services.System;
using System;
using System.ComponentModel;
using System.Numerics;

namespace NEP5
{
    public class NEP5 : SmartContract
    {
        [DisplayName("transfer")]
        public static event Action<byte[], byte[], BigInteger> Transferred;

        private static readonly byte[] Owner = "AdhU77G7mbhnYgTe12ZAwFTomoP2MhsTbi".ToScriptHash(); //Owner Address
        private static readonly BigInteger TotalSupplyValue = 10000000000000000;

        public static object Main(string method, object[] args)
        {
            if (Runtime.Trigger == TriggerType.Verification)
            {
                return Runtime.CheckWitness(Owner);
            }
            else if (Runtime.Trigger == TriggerType.Application)
            {
                var callscript = ExecutionEngine.CallingScriptHash;

                if (method == "balanceOf") return BalanceOf((byte[])args[0]);

                if (method == "decimals") return Decimals();

                if (method == "deploy") return Deploy();

                if (method == "name") return Name();

                if (method == "symbol") return Symbol();

                if (method == "supportedStandards") return SupportedStandards();

                if (method == "totalSupply") return TotalSupply();

                if (method == "transfer") return Transfer((byte[])args[0], (byte[])args[1], (BigInteger)args[2], callscript);
            }
            return false;
        }

        [DisplayName("balanceOf")]
        public static BigInteger BalanceOf(byte[] account)
        {
            if (account.Length != 20)
                throw new InvalidOperationException("The parameter account SHOULD be 20-byte addresses.");
            StorageMap asset = Storage.CurrentContext.CreateMap(nameof(asset));
            return asset.Get(account).AsBigInteger();
        }
        [DisplayName("decimals")]
        public static byte Decimals() => 8;

        private static bool IsPayable(byte[] to)
        {
            var c = Blockchain.GetContract(to);
            return c == null || c.IsPayable;
        }

        [DisplayName("deploy")]
        public static bool Deploy()
        {
            if (TotalSupply() != 0) return false;
            StorageMap contract = Storage.CurrentContext.CreateMap(nameof(contract));
            contract.Put("totalSupply", TotalSupplyValue);
            StorageMap asset = Storage.CurrentContext.CreateMap(nameof(asset));
            asset.Put(Owner, TotalSupplyValue);
            Transferred(null, Owner, TotalSupplyValue);
            return true;
        }

        [DisplayName("name")]
        public static string Name() => "VM243"; //name of the token

        [DisplayName("symbol")]
        public static string Symbol() => "vm"; //symbol of the token

        [DisplayName("supportedStandards")]
        public static string[] SupportedStandards() => new string[] { "NEP-5", "NEP-7", "NEP-10" };

        [DisplayName("totalSupply")]
        public static BigInteger TotalSupply()
        {
            StorageMap contract = Storage.CurrentContext.CreateMap(nameof(contract));
            return contract.Get("totalSupply").AsBigInteger();
        }
#if DEBUG
        [DisplayName("transfer")] //Only for ABI file
        public static bool Transfer(byte[] from, byte[] to, BigInteger amount) => true;
#endif
        //Methods of actual execution
        private static bool Transfer(byte[] from, byte[] to, BigInteger amount, byte[] callscript)
        {
            //Check parameters
            if (from.Length != 20 || to.Length != 20)
            {
                Runtime.Notify("The parameters from and to SHOULD be 20-byte addresses.");
                throw new InvalidOperationException("The parameters from and to SHOULD be 20-byte addresses.");
            }  
                
            if (amount <= 0)
            {
                Runtime.Notify("The parameter amount MUST be greater than 0.");
                throw new InvalidOperationException("The parameter amount MUST be greater than 0.");
            }
                
            if (!IsPayable(to))
                return false;
            if (!Runtime.CheckWitness(from) && from.AsBigInteger() != callscript.AsBigInteger())
            {
                Runtime.Notify("The parameters from and to SHOULD be 20-byte addresses.");
                return false;
            }
                
            StorageMap asset = Storage.CurrentContext.CreateMap(nameof(asset));
            var fromAmount = asset.Get(from).AsBigInteger();
            Runtime.Notify(fromAmount);
            if (fromAmount < amount) {
                Runtime.Notify("not enought tokens");
                return false;
            }
                
            if (from == to)
                return true;

            //Reduce payer balances
            if (fromAmount == amount)
                asset.Delete(from);
            
            else
                asset.Put(from, fromAmount - amount);
                Runtime.Notify("from balance recalculate successfully");

            //Increase the payee balance
            var toAmount = asset.Get(to).AsBigInteger();

            asset.Put(to, toAmount + amount);
            Runtime.Notify("recalculate successfully");

            Transferred(from, to, amount);
            Runtime.Notify("from");
            Runtime.Notify(from);
            Runtime.Notify("to:");
            Runtime.Notify(to);
            Runtime.Notify("amount:");
            Runtime.Notify(amount);
            return true;
        }
    }
}

@shargon
Copy link
Member Author

shargon commented Nov 21, 2019

@superboyiii Do you think that your issue is related to this fix?

@superboyiii
Copy link
Member

@shargon No, I don't think it's related to your fix. It's compatible. I assume these are some issues from other place.

@shargon
Copy link
Member Author

shargon commented Nov 21, 2019

I assume these are some issues from other place.

Could you open a new issue for that?

@erikzhang erikzhang merged commit 52a699a into neo-project:master-2.x Nov 25, 2019
@erikzhang erikzhang deleted the fix-base58-2x branch November 25, 2019 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to tag confirmed bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants