[CLI] Rewrite solc --link to use LinkerObject

[cameel]

Extracted from #10297 (comment):

@axic

Why do we have this messy code here and why isn't this part of LinkerObject? Also solc-js has this messy code duplicated.

I would prefer a clear way of adding a LinkerObject::parse(bytes) feature and unit testing, and using LinkerObject::parse(bin).link(addresses) in the cli.

You can take inspiration from https://github.com/ethereum/solc-js/blob/master/linker.js (see findLinkReferences).

[Uttam-Singhh]

Is this issue solved or I can work on it??

[cameel]

No, no one is working on it right now so if you want to try, feel free :)

[Uttam-Singhh]

Hey, @cameel I am new to this.
It would be great if you can guide me on how to get started and what I exactly have to do!

[cameel]

Great! Not sure how familiar you are with linking and library addresses in Solidity. If it's a new topic for you, I'd suggest to start by reading the Library Linking section and the chapter on Libraries in the docs.

Here's a general rundown of everything you need for this particular task:

• First, to get an idea what this is all about, try to compile a contract that uses a library and see the resulting bytecode. For example

  solc test/libsolidity/semanticTests/libraries/stub.sol --bin

  ======= test/libsolidity/semanticTests/libraries/stub.sol:C =======
  Binary:
  608060405234801561001057600080fd5b50610217806100206000396000f3fe608060405234801561001057600080fd5b506004361061002b5760003560e01c8063e420264a14610030575b600080fd5b61004a6004803603810190610045919061011a565b610060565b60405161005791906101a5565b60405180910390f35b600073__$b3157e226f2c4dddae135e6cab4ed4e747$__63b3de648b836040518263ffffffff1660e01b8152600401610099919061018a565b60206040518083038186803b1580156100b157600080fd5b505af41580156100c5573d6000803e3d6000fd5b505050506040513d601f19601f820116820180604052508101906100e99190610143565b9050919050565b6000813590506100ff816101ca565b92915050565b600081519050610114816101ca565b92915050565b60006020828403121561012c57600080fd5b600061013a848285016100f0565b91505092915050565b60006020828403121561015557600080fd5b600061016384828501610105565b91505092915050565b610175816101c0565b82525050565b610184816101c0565b82525050565b600060208201905061019f600083018461017b565b92915050565b60006020820190506101ba600083018461016c565b92915050565b6000819050919050565b6101d3816101c0565b81146101de57600080fd5b5056fea264697066735822122020a8bc2e01963760c213821310bc9bea51a3cfcc7dc46771171a6028adddb66b64736f6c63430008040033
  
  // $b3157e226f2c4dddae135e6cab4ed4e747$ -> test/libsolidity/semanticTests/libraries/stub.sol:L
  
  ======= test/libsolidity/semanticTests/libraries/stub.sol:L =======
  Binary:
  6101d4610053600b82828239805160001a607314610046577f4e487b7100000000000000000000000000000000000000000000000000000000600052600060045260246000fd5b30600052607381538281f3fe73000000000000000000000000000000000000000030146080604052600436106100355760003560e01c8063b3de648b1461003a575b600080fd5b81801561004657600080fd5b50610061600480360381019061005c91906100a1565b610077565b60405161006e91906100d9565b60405180910390f35b6000818261008591906100f4565b9050919050565b60008135905061009b81610187565b92915050565b6000602082840312156100b357600080fd5b60006100c18482850161008c565b91505092915050565b6100d38161014e565b82525050565b60006020820190506100ee60008301846100ca565b92915050565b60006100ff8261014e565b915061010a8361014e565b9250817fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff048311821515161561014357610142610158565b5b828202905092915050565b6000819050919050565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052601160045260246000fd5b6101908161014e565b811461019b57600080fd5b5056fea26469706673582212203a397998c84d74f23a1af49ff538f5a353039460fe00aa622d24ba9035d8743664736f6c63430008040033
  

  • Note the __$b3157e226f2c4dddae135e6cab4ed4e747$__ part inside C's bytecode (you need to scroll to see it above). It's a placeholder for a library address. Linking in Solidity simply means inserting an actual library address in that spot. It takes up exactly 40 characters in the hex encoding that represent 20 bytes in the decoded bytecode.
  • To make this more apparent the compiler lists all the placeholders below the bytecode, which tells you the exact name of the library and the file.
• Try telling the compiler where that library has been deployed on the blockchain:

  solc test/libsolidity/semanticTests/libraries/stub.sol --bin --libraries test/libsolidity/semanticTests/libraries/stub.sol:L=0x1234567890123456789012345678901234567890

  • This way you told the compiler to build and immediately link the contract (all in one operation).
  • Note that that the placeholder has disappeared from the produced bytecode. It has been replaced with the address you gave.
• You can also perform linking on an already compiled binary using the --link option:

  solc test/libsolidity/semanticTests/libraries/stub.sol --bin > bytecode.bin
  solc bytecode.bin --link --libraries test/libsolidity/semanticTests/libraries/stub.sol:L=0x1234567890123456789012345678901234567890

  • Note that in the --link mode the compiler modifies the bytecode in place. To see the result you need to open bytecode.bin.
• The issue here is that these two methods are implemented separately.
  • The first one uses LinkerObject::link(). See LinkerObject.h/LinkerObject.cpp.
  • The second uses CommandLineInterface::link(). See CommandLineInterface.cpp.
• The goal is to rewrite CommandLineInterface::link() to use LinkerObject::link() internally.
• Note an important detail: LinkerObject operates on binary bytecode (that's what LinkerObject::bytecode stores) while the binary CommandLineInterface::link() gets as input is hex-encoded - except for the placeholders. So you need to detect the placeholders and decode everything except them. Only then you can create a LinkerObject and use its link() method.

If you run into problems with the the implementation, please drop by on the solidity-dev channel. During the week you can find me and other developers there (though note that this Monday is a holiday so probably best to try on Tuesday :)).

[Uttam-Singhh]

Thank you! so much @cameel!
I will start working on this as soon as possible...

[cameel]

@Uttam-Singhh Are you still working on this issue? Do you need any help?

[Uttam-Singhh]

Hey @cameel
Sorry, I was not able to work on this, I tried to learn the concepts but I think so I need to learn more to work on this.
I am unassigning myself. Sorry for the inconvenience!
Looking forward to working on more issues in the future.
Thank you so much for you guidance @cameel!

[cameel]

No worries! If you want, you could try a different task, maybe it'll be easier for a start. Here are some possibilities:

• [yulopti] interactively run one iteration of the sequence between [] from fullsuite #10812 - this is a simple tool that consists of a single .cpp file. Should be pretty easy.
• [CLI] Report errors for command line options that have no effect #11629 - in this there are will probably be a lot of changes but they should be all simple (just adding checks and printing errors) and you can also just do them one at a time to make PRs simpler.
• [CLI] Allow for printing output without headings (such as bytecode) #10275
• Make usage of quotes in errors consistent #7578

[Uttam-Singhh]

Thank you so much @cameel. I will have a look at them!

[ryzheboka]

Hi, can I work on this issue? Can you please check my understanding of the changes? I would need to rewrite CommandLineInterface::link(), so it reuses code from LinkerObject::link() for linking already compiled code. The code in LinkerObject::link() is already used for linking with the option --libraries. What do I do with the placeholders while translating the hex-encoded input of CommandLineInterface into binary?

[cameel]

Please take a look at #10308 (comment). It describes in detail what needs to be done.

What do I do with the placeholders while translating the hex-encoded input of CommandLineInterface into binary?

It does not matter all that much because LinkerObject does not really look at them and instead uses the linkReferences map to find the right place in the bytecode to overwrite. You need to find them and fill the linkReferences map but after that you can just replace them with anything that will not cause an error during hex decoding.

One thing to keep in mind though is that after linking there might still remain some unlinked references if they were not given in --libraries. When that's the case, you will need to restore them to what they originally were.

[ryzheboka]

I'm sorry, I won't be able to work on the issue. I had problems building solidity on my computer, and I could't resolve them.

[cameel]

Recently a user requested that we add config for one of the online IDEs, claiming that it would be a good way for contributors to easily get a working environment. It was #12112. I haven't personally tried that IDE so I can't say if it would work but maybe you'd like to try? It might let you work on this without having to fight with the build system. And I'd be interesting to know if it's really something we could recommend to contributors.

[ryzheboka]

Yes, I will gladly try using the online IDE.

[cameel]

Great. Let me know if you need any help. Maybe @nickytonline would be able to chime in too since he's using that IDE.

[nickytonline]

@ryzheboka, I'm new to the Solidity project, but am pretty familiar with Gitpod, so if you have questions, don't hesitate.

[ryzheboka]

Building on Gitpod with the config from #12112 worked! I need to get more comfortable with Gitpod, and I will work on the issue from there.

[nickytonline]

@ryzheboka, I've updated my branch to include the Ethereuem PPA and when I loaded up Gitpod today, it seems all good. Let me know if you still have issues.

[ryzheboka]

Thanks @nickytonline, it worked with the new gitpod.yml! (Still only with -DUSE_Z3=OFF)
Now, I have the following problem with testing, that seems to be configuration related:
"./scripts/../test/cmdlineTests.sh: line 185: /dev/stdin: Permission denied
Incorrect output on stderr received. Expected:
"file_not_found.sol" is not found.
But got:

When running /workspace/solidity/build/solc/solc file_not_found.sol </dev/stdin"

[cameel]

The script asks you to update the file if expectations do not match. If should just fail if there's no terminal - that's how it works in our CI. If that's not the case, you could try to explicitly take away its input stream with something like this:

cmdlineTests.sh < /dev/null

Of if you call it via tests.sh, you could add < /dev/null to that script invocation instead.

[nickytonline]

Thanks @nickytonline, it worked with the new gitpod.yml! (Still only with -DUSE_Z3=OFF) Now, I have the following problem with testing, that seems to be configuration related: "./scripts/../test/cmdlineTests.sh: line 185: /dev/stdin: Permission denied Incorrect output on stderr received. Expected: "file_not_found.sol" is not found. But got:

When running /workspace/solidity/build/solc/solc file_not_found.sol </dev/stdin"

z3 should be the correct version now if you grab latest of my branch.

[ryzheboka]

Z3 works now, I forgot to save the changed .yml file in gitpod. < /dev/null helped too. Could you please help with the following message?

"Error loading VM from libevmone.so:
libevmone.so: cannot open shared object file: No such file or directory
Unable to find libevmone.so. Please disable semantics tests with --no-semantic-tests or provide a path using --vm ."

I tried running the tests with --no-smt, but got the same message. It was not possible to run tests.sh with the argument "-no-semantic-tests". All the tests till the message run successfully.

[cameel]

The --no-semantic-tests flag is supported by the soltest and isoltest executables but probably not by scripts/tests.sh. For your task, again semantic tests are not relevant but they are very important overall so I'd recommend installing evmone anyway so that you can run them.

So to solve this, please check if you can install a package called evmone or libevmone. If not, you can download pre-built binary from https://github.com/ethereum/evmone/releases/ and then put under deps/lib/ in the repository so that soltest can find it. See also Contributing > Running the Tests if you need more info.

[ryzheboka]

Thanks, semantic tests run successfully. The documentation says "Running Ewasm tests is disabled by default", however, I get the following error:

Error loading VM from libhera.so:
libhera.so: cannot open shared object file: No such file or directory
Unable to find libhera.so.

Should I repeat the steps for evmone for hera too?

[cameel]

Yeah, it's disabled by default but seems that tests.sh tries to be comprehensive and does an ewasm run as well:

solidity/scripts/tests.sh

Line 140 in 7334420

[ "${vm}" = "byzantium" ] && [ "${optimize}" = "" ] && EWASM_ARGS="--ewasm"

You don't really need it in this task so I recommend disabling it. The easiest way to do this would be to just locally comment out that line in your branch. If you want you can also submit a PR that adds a --no-ewasm option to the script. It already supports --no-smt and the new option would be pretty similar. It would also not hurt to have --no-semantic-tests there to have a full set.

[ryzheboka]

Next weeks will be very full and stressful for me. I probably won't be able to finish the testing, at least not in next weeks. I'm very sorry!

[cameel]

No worries. Take your time. There is no deadline for this task so if you want to just come back to it later when you have more time that's not a problem for us :)

[bmoore01]

I've created a PR that hopefully resolves this issue #12756

[cameel]

@bmoo0 Thanks! I'm going to reassign the issue to you then.

[Marenz]

Current status is that there is a PR implementing this but it needs some cleanup work

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[github-actions]

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.