Conversation
stdlib/assembly.sol
Outdated
| assembly { | ||
| // allocates a memory block | ||
| // TODO: use memoryguard | ||
| function alloc(size) -> ptr { |
There was a problem hiding this comment.
| function alloc(size) -> ptr { | |
| function allocate(size) -> ptr { |
There was a problem hiding this comment.
I think actually these memory functions may be better as being part of the inline assembly dialect (as discussed on #10399).
There was a problem hiding this comment.
Yes, but I think it would be good to also have the translation between the two dialects available.
stdlib/assembly.sol
Outdated
| @@ -0,0 +1,53 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
There was a problem hiding this comment.
Could be MIT too. The goal is to avoid license baggage for users. They may want to keep their deployed codebase with less restrictions than GPL-3.0.
There was a problem hiding this comment.
One thing I really don't like about Apache license is this bit:
You must cause any modified files to carry prominent notices stating that You changed the files
I.e. it requires the source to be annotated with extensive info about who changed it. I think that such metadata belongs in the version control, not in the file. It would be pretty tedious to have to keep it in the source.
There was a problem hiding this comment.
I think it just means stating it was changed and not what has been. Though in practice this does not take place in the original source repo, just in forks.
Also note Rust uses a dual MIT/Apache2 license and a large part of the Ethereum ecosystem uses Apache2.
stdlib/globals.sol
Outdated
| assembly { | ||
| // Prepare input | ||
| // We are using the free memory pointer. | ||
| let input := mload(0x40) |
There was a problem hiding this comment.
Move this into a function?
There was a problem hiding this comment.
So far these files were independently working, and the assembly file of course does not work yet. But yes, eventually it should use the assembly helpers.
stdlib/globals.sol
Outdated
| mstore(output, 0) | ||
|
|
||
| // Call the precompile | ||
| let ret := staticcall(gas(), 1, input, 128, output, 32) |
There was a problem hiding this comment.
Doesn't ecrecover need more error checks?
There was a problem hiding this comment.
This is currently a 1-on-1 mapping to ecrecover in the language. If we do semantic changes, then we can consider an empty output as failure and revert.
stdlib/globals.sol
Outdated
| mstore(input, hash) | ||
| mstore(add(input, 32), v) | ||
| mstore(add(input, 64), r) | ||
| mstore(add(input, 96), s) |
There was a problem hiding this comment.
Could also use bytes manipulation (as per discussions in #9829):
bytes input = new bytes(96);
input[0..32] = bytes32(bytes8(v));
input[32..64] = r;
input[64..96] = s;
Or
bytes input = abi.encodePacked(bytes32(bytes8(v)), r, s);
If we manage to improve memory handling 😉
|
I wonder if we should just start committing stdlib under |
stdlib/blocktx.sol
Outdated
| function blockhash(uint blockNumber) returns (bytes32 ret) { | ||
| assembly { | ||
| ret := blockhash(blockNumber) | ||
| } | ||
| } |
There was a problem hiding this comment.
Just a heads up before we drop the source in the repo - I think that the piece of code as prominent as the standard library should really follow our own coding style :)
| function blockhash(uint blockNumber) returns (bytes32 ret) { | |
| assembly { | |
| ret := blockhash(blockNumber) | |
| } | |
| } | |
| function blockhash(uint blockNumber) returns (bytes32 ret) { | |
| assembly { | |
| ret := blockhash(blockNumber) | |
| } | |
| } |
EDIT: turns out the _ prefix for parameters is not in our Solidity style guidelines. So only indents are wrong.
There was a problem hiding this comment.
Makes sense. Do we have any good guide for variable names (i.e. for ret)?
stdlib/utils.sol
Outdated
| function min<T>(T a, T b) pure internal returns (T ret) { | ||
| ret = (a < b) ? a : b; | ||
| } | ||
|
|
||
| function max<T>(T a, T b) pure internal returns (T ret) { | ||
| ret = (a > b) ? a : b; | ||
| } | ||
|
|
||
| function abs<T: T.isSigned>(T a) pure internal returns (T ret) { | ||
| ret = (a < 0) ? -a : a; | ||
| } |
stdlib/globals.sol
Outdated
| //import sha256 from std; | ||
| //import chainid from std; | ||
|
|
||
| contract C{ |
There was a problem hiding this comment.
| contract C{ |
There was a problem hiding this comment.
Yup, this was before free functions :)
stdlib/blocktx.sol
Outdated
| } | ||
| } | ||
|
|
||
| library Block { |
There was a problem hiding this comment.
Instead of libraries, we can split these into different files and import them based on file names.
There was a problem hiding this comment.
Yes, I think we should split this in many small files.
There was a problem hiding this comment.
(mainly to test out if the way we plan to do namespacing in the future works out well or not)
stdlib/errors.sol
Outdated
| uint constant PanicResourceError = 0x41; | ||
| uint constant PanicInvalidInternalFunction = 0x51; | ||
|
|
||
| error Panic(uint code); |
There was a problem hiding this comment.
Should we natspec the stdlib?
|
@axic wen stdlib |
|
Pushed here a new approach, where a compiler flag enables the stdlib. This way perhaps we can merge these 3 precompiles as a start. |
solstdlib/solstdlib.h.in
Outdated
| { | ||
|
|
||
| static std::map<std::string, std::string> sources = { | ||
| { "std/precompiles.sol", precompiles }, |
There was a problem hiding this comment.
Didn't find a good cmake way to generate this file 👁️
| function sha256(bytes memory input) returns (bytes32 ret) { | ||
| assembly { | ||
| let success := staticcall(gas(), 2, add(input, 32), mload(input), 0, 32) | ||
| if iszero(success) { revert(0, 0) } |
There was a problem hiding this comment.
These precompiles currently differ in two ways from the IR:
- only support newer EVM versions where passing all gas is possible (would need Introduce solidity.evmVersion() to query the target EVM #10283 to support conditional options)
- it does not pass through the revert data (this should be fixed before merging)
|
This pull request is stale because it has been open for 14 days with no activity. |
|
Removing the stale label here - but @nikola-matic, you wanted to check and report the state of this, s.t. we can decide all remaining open questions, if any (I guess it's mainly stuff like the name used in importing and such?), and actually move this forward, right? [For the record: this only occurs in the roadmap as a "future" roadmap item - but that is supposed to mean to be finished with this - this PR is only the very first small step towards moving everything that can be moved and the entire thing will both take a lot of time and require generics - but that doesn't mean that we shouldn't still start and do what we can do "now" or at least soonish - no immediate hurry, but we also shouldn't just keep this open and go to waste forever] |
|
This pull request is stale because it has been open for 14 days with no activity. |
We can add the |
|
|
|
Decision from the call: we'd prefer a special syntax for the stdlib imports: |
Changes to |
|
I did cross-post there already. Small changes in the parser will of course be needed (the stdlib import will use an identifier path) but this will all be behind an experimental pragma so non-breaking. And I don't think we'll be keeping the ANTLR grammar up to date with the experimental changes. @ekpyron already thinks that even trying to keep it up to date as is is too much of a distraction. |
Ah, so we're changin the pragma directive as well. Should this be implemented separately as a prerequisite for stdlib, and then this PR updated respectively? |
|
I think separately, because we want it done and merged in the very near future. It should not be held back by discussions of other changes in this PR. |
|
@ekpyron said on the call that anyone not busy with other tasks could start working on that already. We'll discuss further steps next week. |
|
Yeah, we won't maintain an antlr grammar for the experimental version of the language - we can do that once we have stabilized it :-). And yes, this PR will be rebased on top of a generic "pragma experimental solidity-next;" PR, which we'll have soon (after that we can drop the
"That" being the |
|
Closing in favour of #14249 |
Part of #10282.
TODO:
std/), check if its contents match the built-in contents and don't do anything. Otherwise warn.If the three points are combined, you can export the standard library to a folder and importing from that folder would do the same as importing the built-in standard library.
The contents of the standard library are of course version-dependent.