search

stellar / stellar-core

Reference implementation for the peer-to-peer agent that manages the Stellar network.
https://www.stellar.org
Other
3.12k stars 969 forks source link

ContractIDs not pretty printed properly #4184

Open MonsieurNicolas opened 8 months ago

MonsieurNicolas commented 8 months ago

It looks like https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0023.md is not implemented in core.

When converting an SCAddress into a string, it just displays them as hex instead of a C... address.

For example, operations (when logging) are stringified as:

"Operation": {
        "sourceAccount": "GCLA2E3LQDPAPJLHYDMB5R65ASGLNXWGJCX4TX7XA75C7VTJ7Y2OTZXA",
        "body": {
            "type": "INVOKE_HOST_FUNCTION",
            "invokeHostFunctionOp": {
                "hostFunction": {
                    "type": "HOST_FUNCTION_TYPE_INVOKE_CONTRACT",
                    "invokeContract": {
                        "contractAddress": {
                            "type": "SC_ADDRESS_TYPE_CONTRACT",
                            "contractId": "2ab7f467d192fcb2aefa138e0aae1f596216692d5e3d631a2c2ee81e263ab83f"
                        },
                        "functionName": "set_price",
                        "args": [
                            {
                                "type": "SCV_ADDRESS",
                                "address": {
                                    "type": "SC_ADDRESS_TYPE_ACCOUNT",
                                    "accountId": "GCLA2E3LQDPAPJLHYDMB5R65ASGLNXWGJCX4TX7XA75C7VTJ7Y2OTZXA"
                                }
                            },
                            {

where it should be something like:

                        "contractAddress": {
                            "type": "SC_ADDRESS_TYPE_CONTRACT",
                            "contractId": "CSOMEKEYDPAPJLHYDMB5R65ASGLNXWGJCX4TX7XA75C7VTJ7Y2OTZXA"
                        },

This also impacts other places where we display addresses:

MonsieurNicolas commented 8 months ago

@leighmcculloch I imagine that we're consistently using "C" strings with tooling outside of core?

leighmcculloch commented 8 months ago

Yup, we're consistently using C strkeys in the soroban-env-* crates and all tooling. There was a bug in env where we still used hashes in one place and it was just fixed.

The Rust implementation is here: https://github.com/stellar/rs-stellar-strkey/blob/c5a374eb10dd97492340f8cda72a4a96562e55c2/src/strkey.rs#L175-L225

MonsieurNicolas commented 8 months ago

good -- we want people to use strkeys to avoid sad typos, so only this issue should be left then

MonsieurNicolas commented 2 months ago

I think I found more occurences of this.

When decoding TransactionMeta using the print-xdr command, I get contractID in hex:

        "sorobanMeta": {
                "ext": {
                    "v": 0
                },
                "events": [
                    {
                        "ext": {
                            "v": 0
                        },
                        "contractID": "9a1c07d063c8cff8ba513ca036633251658ef3297dfa1acef951e7e5f5ee0c78",
                        "type": "CONTRACT",
                        "body": {

This is when decoding

AAAAAwAAAAAAAAACAAAAAwATRCIAAAAAAAAAAJrxsoyNwlOxn1SvTaq4az+yDOErBq47ezvx8YNLvl2YAAAAAK/bo9wABOelAAABEQAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAMAAAAAABNEIQAAAABmzgUAAAAAAAAAAAEAE0QiAAAAAAAAAACa8bKMjcJTsZ9Ur02quGs/sgzhKwauO3s78fGDS75dmAAAAACv26PcAATnpQAAARIAAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAIAAAAAAAAAAAAAAAAAAAADAAAAAAATRCIAAAAAZs4FBQAAAAAAAAABAAAAHgAAAAAAE0QiAAAACXkMsXSkMB7YudB79Bw0jUGKT1bK8p6GY90RnWoxCnxjADLoIQAAAAAAAAAAABNEIgAAAAYAAAAAAAAAAdO4GjbQ/5XIOxrdY/FmiAJvg39WqDql5A102xPK8n59AAAAFAAAAAEAAAATAAAAANmJ1zygMll5UFfgwljl2ukoZkuHdndRDoo7GBOcylO2AAAAAAAAAAAAAAAAABNEIgAAAAk7hyWgcJfdtMPrO8kugadxc7lvqdv6xYIrawmT6UR8nQAy6CEAAAAAAAAAAAATRCIAAAAJr+h+5fBdyNeL4jkFigqlJkCw9nNqbr3nrZnOTcFWPL8AMughAAAAAAAAAAAAE0QiAAAABgAAAAAAAAABmhwH0GPIz/i6UTygNmMyUWWO8yl9+hrO+VHn5fXuDHgAAAAPAAAABkNPTkZJRwAAAAAAAQAAABEAAAABAAAABgAAAA8AAAAIbHBfdG9rZW4AAAASAAAAAR4yAC5cyacCzN2cbb/HvABf3Xj77Xu8q5h4fXRVaTyAAAAADwAAAAdtYW5hZ2VyAAAAABIAAAAAAAAAAJrxsoyNwlOxn1SvTaq4az+yDOErBq47ezvx8YNLvl2YAAAADwAAAA5tYXhfY29tcGxleGl0eQAAAAAAAwAAAAcAAAAPAAAACG1pbl9ib25kAAAACgAAAAAAAAAAAAAAAAAAAGQAAAAPAAAACm1pbl9yZXdhcmQAAAAAAAoAAAAAAAAAAAAAAAAAAABkAAAADwAAAAVvd25lcgAAAAAAABIAAAABgF+JXP8DjWwDwgknIqxybwxpLmGmzZGf5BcctnuxOscAAAAAAAAAAAATRCIAAAAJckVRxF67aPW/FJVnGB4OT2JVS+8s2N1x1MQ3AIDI0asAMughAAAAAAAAAAAAE0QiAAAABgAAAAAAAAAB07gaNtD/lcg7Gt1j8WaIAm+Df1aoOqXkDXTbE8ryfn0AAAADAAAAAwAAAAEAAAASAAAAAAAAAACa8bKMjcJTsZ9Ur02quGs/sgzhKwauO3s78fGDS75dmAAAAAAAAAAAABNEIgAAAAYAAAAAAAAAAdO4GjbQ/5XIOxrdY/FmiAJvg39WqDql5A102xPK8n59AAAAAwAAAAIAAAABAAAACgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAATRCIAAAAJd7wC3mGBfw0IYiZf4EZSbH+qlNkwnbI8RPk8ujhMbu8AMughAAAAAAAAAAAAE0QiAAAABgAAAAAAAAABgF+JXP8DjWwDwgknIqxybwxpLmGmzZGf5BcctnuxOscAAAARAAAAAQAAAAIAAAAPAAAAB3Rva2VuX2EAAAAAEgAAAAFEX034luymThqJmmqoVg92Nca3EmCxg1VVRABPKdo99AAAAA8AAAAHdG9rZW5fYgAAAAASAAAAAdeSi3LCcDzP6vfrn/TvTVBKVai5efybRQ6iyEK00c5hAAAAAQAAABIAAAAB07gaNtD/lcg7Gt1j8WaIAm+Df1aoOqXkDXTbE8ryfn0AAAAAAAAAAAATRCIAAAAGAAAAAAAAAAGaHAfQY8jP+LpRPKA2YzJRZY7zKX36Gs75Uefl9e4MeAAAAAMAAAAAAAAAAQAAABIAAAAAAAAAAJrxsoyNwlOxn1SvTaq4az+yDOErBq47ezvx8YNLvl2YAAAAAAAAAAAAE0QiAAAACVVG4kalRrMdRLn1UemqmmmeFdWtG7hWS5WZsqhKbCwNADLoIQAAAAAAAAAAABNEIgAAAAYAAAAAAAAAAdO4GjbQ/5XIOxrdY/FmiAJvg39WqDql5A102xPK8n59AAAAAwAAAAEAAAABAAAACgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAATRCIAAAAJh21BcH3wUnsChafdza6JqL/y39bkt9EVW39GXdUhLTMAMughAAAAAAAAAAAAE0QiAAAACSt4huRNeOt0zNlHIQd8cY9Hy5hnFjcGyIWoueDVxg9tADLoIQAAAAAAAAAAABNEIgAAAAn/KGQBfI6k+lc2miR1PcGCWsg7UAwXTx8E/gVu2kYSegAy6CEAAAAAAAAAAAATRCIAAAAGAAAAAAAAAAGaHAfQY8jP+LpRPKA2YzJRZY7zKX36Gs75Uefl9e4MeAAAAAMAAAADAAAAAQAAAAAAAAABAAAAAAAAAAAAE0QiAAAABgAAAAAAAAAB07gaNtD/lcg7Gt1j8WaIAm+Df1aoOqXkDXTbE8ryfn0AAAAPAAAABkNPTkZJRwAAAAAAAQAAABEAAAABAAAACgAAAA8AAAANZmVlX3JlY2lwaWVudAAAAAAAABIAAAAAAAAAAJrxsoyNwlOxn1SvTaq4az+yDOErBq47ezvx8YNLvl2YAAAADwAAABhtYXhfYWxsb3dlZF9zbGlwcGFnZV9icHMAAAAGAAAAAAAAJxAAAAAPAAAAFm1heF9hbGxvd2VkX3NwcmVhZF9icHMAAAAAAAYAAAAAAAAnEAAAAA8AAAAQbWF4X3JlZmVycmFsX2JwcwAAAAYAAAAAAAATiAAAAA8AAAAJcG9vbF90eXBlAAAAAAAAAwAAAAAAAAAPAAAAC3NoYXJlX3Rva2VuAAAAABIAAAABHjIALlzJpwLM3Zxtv8e8AF/dePvte7yrmHh9dFVpPIAAAAAPAAAADnN0YWtlX2NvbnRyYWN0AAAAAAASAAAAAZocB9BjyM/4ulE8oDZjMlFljvMpffoazvlR5+X17gx4AAAADwAAAAd0b2tlbl9hAAAAABIAAAABRF9N+Jbspk4aiZpqqFYPdjXGtxJgsYNVVUQATynaPfQAAAAPAAAAB3Rva2VuX2IAAAAAEgAAAAHXkotywnA8z+r365/0701QSlWouXn8m0UOoshCtNHOYQAAAA8AAAANdG90YWxfZmVlX2JwcwAAAAAAAAYAAAAAAAAD6AAAAAAAAAAAABNEIgAAAAYAAAAAAAAAAZocB9BjyM/4ulE8oDZjMlFljvMpffoazvlR5+X17gx4AAAAAwAAAAEAAAABAAAACgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAATRCIAAAAJ2MdPbuT22sB+nCA1Rc1GWkeRnIDWn2b0ReI9K5WFttgAMughAAAAAAAAAAAAE0QiAAAABgAAAAAAAAABmhwH0GPIz/i6UTygNmMyUWWO8yl9+hrO+VHn5fXuDHgAAAAUAAAAAQAAABMAAAAACteTcwa/ys2dr1TmboejCLFJ2yxos9RIRPoMVfh9PigAAAAAAAAAAAAAAAAAE0QiAAAABgAAAAAAAAAB07gaNtD/lcg7Gt1j8WaIAm+Df1aoOqXkDXTbE8ryfn0AAAADAAAABAAAAAEAAAAAAAAAAQAAAAAAAAADABNEIQAAAAYAAAAAAAAAAYBfiVz/A41sA8IJJyKscm8MaS5hps2Rn+QXHLZ7sTrHAAAAAwAAAAIAAAABAAAAEAAAAAEAAAAAAAAAAAAAAAEAE0QiAAAABgAAAAAAAAABgF+JXP8DjWwDwgknIqxybwxpLmGmzZGf5BcctnuxOscAAAADAAAAAgAAAAEAAAAQAAAAAQAAAAEAAAASAAAAAdO4GjbQ/5XIOxrdY/FmiAJvg39WqDql5A102xPK8n59AAAAAAAAAAAAE0QiAAAACampAoo2J335VWKxbWLgI06GZcLZ+j7pjtHCf+ap0omBADLoIQAAAAAAAAAAABNEIgAAAAlsy3CPBdIbN3VwVWw2BdHXKBl0e3cfgl92surXXvJNEQAy6CEAAAAAAAAAAAATRCIAAAAJPRWE3NUtY4aqb6KYEU/AmgK7wUG5xN7twqKyJXlZP2sAMughAAAAAAAAAAAAE0QiAAAACTsDK54+RuiL6ZnPd/y6lfBQYZwaq3iIhmSokd3HfChGADLoIQAAAAAAAAAAABNEIgAAAAYAAAAAAAAAAdO4GjbQ/5XIOxrdY/FmiAJvg39WqDql5A102xPK8n59AAAAAwAAAAAAAAABAAAACgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAATRCIAAAAGAAAAAAAAAAEeMgAuXMmnAszdnG2/x7wAX914++17vKuYeH10VWk8gAAAABQAAAABAAAAEwAAAAAjwJOSmggoP9HLgRDdINnD8vaYGdmzp4+7Dmg6DOIdLwAAAAEAAAACAAAADwAAAAhNRVRBREFUQQAAABEAAAABAAAAAwAAAA8AAAAHZGVjaW1hbAAAAAADAAAABwAAAA8AAAAEbmFtZQAAAA4AAAAIWExNUEhPU1QAAAAPAAAABnN5bWJvbAAAAAAADgAAAARYUFNUAAAAEAAAAAEAAAABAAAADwAAAAVBZG1pbgAAAAAAABIAAAAB07gaNtD/lcg7Gt1j8WaIAm+Df1aoOqXkDXTbE8ryfn0AAAAAAAAAAgAAAAMAE0QiAAAAAAAAAACa8bKMjcJTsZ9Ur02quGs/sgzhKwauO3s78fGDS75dmAAAAACv26PcAATnpQAAARIAAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAIAAAAAAAAAAAAAAAAAAAADAAAAAAATRCIAAAAAZs4FBQAAAAAAAAABABNEIgAAAAAAAAAAmvGyjI3CU7GfVK9NqrhrP7IM4SsGrjt7O/Hxg0u+XZgAAAAAsBkE+AAE56UAAAESAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAACAAAAAAAAAAAAAAAAAAAAAwAAAAAAE0QiAAAAAGbOBQUAAAAAAAAAAQAAAAAAAAAEAAAAAAAAAAGaHAfQY8jP+LpRPKA2YzJRZY7zKX36Gs75Uefl9e4MeAAAAAEAAAAAAAAAAgAAAA4AAAAKaW5pdGlhbGl6ZQAAAAAADgAAAB9MUCBTaGFyZSB0b2tlbiBzdGFraW5nIGNvbnRyYWN0AAAAABIAAAABHjIALlzJpwLM3Zxtv8e8AF/dePvte7yrmHh9dFVpPIAAAAAAAAAAAdO4GjbQ/5XIOxrdY/FmiAJvg39WqDql5A102xPK8n59AAAAAQAAAAAAAAACAAAADgAAAAppbml0aWFsaXplAAAAAAAOAAAADlhZSyBMUCB0b2tlbl9hAAAAAAASAAAAAURfTfiW7KZOGomaaqhWD3Y1xrcSYLGDVVVEAE8p2j30AAAAAAAAAAHTuBo20P+VyDsa3WPxZogCb4N/Vqg6peQNdNsTyvJ+fQAAAAEAAAAAAAAAAgAAAA4AAAAKaW5pdGlhbGl6ZQAAAAAADgAAAA5YWUsgTFAgdG9rZW5fYgAAAAAAEgAAAAHXkotywnA8z+r365/0701QSlWouXn8m0UOoshCtNHOYQAAAAAAAAABgF+JXP8DjWwDwgknIqxybwxpLmGmzZGf5BcctnuxOscAAAABAAAAAAAAAAIAAAAOAAAABmNyZWF0ZQAAAAAADgAAAA5saXF1aWRpdHlfcG9vbAAAAAAAEgAAAAHTuBo20P+VyDsa3WPxZogCb4N/Vqg6peQNdNsTyvJ+fQAAABIAAAAB07gaNtD/lcg7Gt1j8WaIAm+Df1aoOqXkDXTbE8ryfn0AAAAA
leighmcculloch commented 2 months ago

It's found in the ContractEvent type:

struct ContractEvent
{
    // We can use this to add more fields, or because it
    // is first, to change ContractEvent into a union.
    ExtensionPoint ext;

    Hash* contractID;
    ContractEventType type;

    union switch (int v)
    {
    case 0:
        struct
        {
            SCVal topics<>;
            SCVal data;
        } v0;
    }
    body;
};

Ref: https://github.com/stellar/stellar-xdr/blob/4ec28d95dd84b109253e22b151314478d6f00522/Stellar-ledger.x#L339-L358

leighmcculloch commented 2 months ago

The Hash* should be a SCAddress*, like: https://github.com/stellar/stellar-xdr/blob/4ec28d95dd84b109253e22b151314478d6f00522/Stellar-ledger-entries.x#L505.

MonsieurNicolas commented 2 months ago

mmm that would be a breaking change as contractID is type 1 not 0. This is annoying: it is plausible that we would want accounts to emit events at some point (as part of unifying data models between classic and Soroban)

leighmcculloch commented 2 months ago

mmm that would be a breaking change as contractID is type 1 not 0.

We can't use the extension point as the ScAddress discriminant, but we could use the extension point to change the Hash to a ScAddress in the next release.

That's backwards compatible, but not forwards compatible. I don't see a way to make the change in a way that's forwards compatible.