Const fields

[g-r-a-n-t]

What is wrong?

There is no way for a user to specify that a field is constant.

This would allow for significant gas saving, since we can store constant values in bytecode, instead of storage.

How can it be fixed

Do something similar to the work done on static strings. The syntax should look something like this: const MINIMUM_LIQUIDITY: u256 = 1000

[rahulbansal16]

I am trying to solve this and I was going through the codebase to understand the implementation. I have figured out the following steps. I have a few doubts. If you can clear those it will help me speed up.

1. Add a Token called CONST in the types.rs.
2. Create the logic inside the tokenizer.rs to parse the const token. I was trying to understand where should I put the statement to parse the const token. I was looking for the All the token list but I was unable to find one. Where are token like contract, type, return are tokenised?
3. Write the logic to parse a token for the const inside the parser.rs. I am unable to understand the purpose of this file? Aren't the tokenizer.rs going to create the tokens from the source code? I see the methods like string_token but they are not called anywhere except the test directory. What is the purpose of those methods?
4. Define a grammar rule inside the Fe.grammar file which will be used to create the const expression. I feel the rule is already added inside the file as contract_field_qual so this step is not required.
5. I am still figuring out the steps after that.

[g-r-a-n-t]

Hi @rahulbansal16, thank you very much for reaching out.

So yeah, as you noted, const is already included in the grammar file and is also included in the parser (see this test).

The grammar is somewhat incorrect, though, in that we can't assign a value to a field, which is of course what we need to do in the case of a constant field.

Currently, we have:

contract_field: [contract_field_qual] NAME ':' type_desc NEWLINE

when we should have:

contract_field: [contract_field_qual] NAME ':' type_desc ['=' expr] NEWLINE

Making this change to the grammar file, updating the parser, and then creating a test to make sure a statement like const foo: u256 = 42 is parsed would be the first step in adding support for constant fields. If you're still interested in helping out, I can provide more guidance. It would definitely be a big help.

[rahulbansal16]

Thanks @g-r-a-n-t I am making changes to add the feature. The PR is #196
It's a WIP. Do take a look if I am taking the correct approach. I have split the grammar rule into two rules.

I was trying to run the test locally on the master branch and I got the following errors for the tests for the test_parser.rs file.

---- test_target stdout ----
thread 'test_target' panicked at 'Test example has wrong format fixtures/parsers/target.ron: "expected 2 parts, got 1"', parser\tests\test_parsers.rs:284:1

I am using a windows system and I am not doing a full build.
I used the following command to run the build and test.
cargo +nightly build
cargo +nightly test --workspace
I used nighty because the rust compiler was asking me to use it.

[cburgdorf]

I am using a windows system and I am not doing a full build.

Sorry to hijack the issue but..Any chance you'd be interested to add CI coverage + Release builds for Windows? I tried to add Windows to our CI suite a while back in #64 but failed to get it to work. None of us is on a windows machine so having someone familiar with windows looking into it would be very much welcome :)

[rahulbansal16]

No worries @cburgdorf, I can start looking into it after I am done with this issue.

[rahulbansal16]

@cburgdorf I am able to download the boost and updating the path. I am getting link errors. I tried the multiple version of the boost and the error is for the boost version 1.69.0 refer to the build logs for more details.

msvcprtd.lib(MSVCP140D.dll) : error LNK2005: "public: __cdecl std::_Locinfo::~_Locinfo(void)" (??1_Locinfo@std@@qeaa@XZ) already defined in libboost_program_options-vc141-mt-sgd-x64-1_69.lib(options_description.obj) [D:\a\fe\fe\target\debug\build\solc-6fd6a06f62b98451\out\build\solc\solc.vcxproj]

libboost_program_options-vc141-mt-sgd-x64-1_69.lib(value_semantic.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MTd_StaticDebug' doesn't match value 'MDd_DynamicDebug' in CommandLineInterface.obj [D:\a\fe\fe\target\debug\build\solc-6fd6a06f62b98451\out\build\solc\solc.vcxproj]

Do you have any idea about fixing them?

[rahulbansal16]

I was also trying with the boost version of 1.73.0 and the error in the build logs is

but it set boost_filesystem_FOUND to FALSE so package "boost_filesystem" is
considered to be NOT FOUND. Reason given by package:
No suitable build variant has been found. * libboost_filesystem-vc141-mt-gd-x64-1_73.lib (vc141, detected vc142, set
Boost_COMPILER to override)
* libboost_filesystem-vc141-mt-s-x32-1_73.lib (32 bit, need 64)

I have few approaches that I am trying. Here Have you encountered these errors before?

[cburgdorf]

@rahulbansal16 Let's discuss this over in #198 I have a rough idea regarding the boost issue.

[cburgdorf]

I'm going to pick this up since constants are quite important and should be part of our beta release.

I just want to make sure I remember correctly what we discussed about constants already. As I understand, we are planning to have three different ways to create them.

1. A const outside of a contract. This type of constant can be used across multiple contracts. Each contract uses the data mechanism to store and load them.

const SOME_CONST: u256 = 1000

contract One:
  pub def multiply_by_const(val: u256) -> u256:
    return val * SOME_CONST

contract Two:
  pub def divide_by_const(val: u256) -> u256:
    return val / SOME_CONST

2. A const field of the contract that is assigned at declaration time. This type of constant can only be used within the contract it is defined. Each contract uses the data mechanism to store and load them.

contract One:
  const SOME_CONST: u256 = 1000
  pub def multiply_by_const(val: u256) -> u256:
    return val * SOME_CONST

contract Two:
  const SOME_CONST: u256 = 2000
  pub def divide_by_const(val: u256) -> u256:
    return val / SOME_CONST

Open question: would this actually need to be read through self.SOME_CONST because technically there's no reason to require self since we aren't reading from contract storage.

3. A const field of the contract that is assigned at contract creation time. This type of constant can only be used within the contract it is defined. We use the setImmutable, loadImmutable mechanism.

Variables declared as immutable are a bit less restricted than those declared as constant: Immutable variables can be assigned an arbitrary value in the constructor of the contract or at the point of their declaration. They cannot be read during construction time and can only be assigned once.

The contract creation code generated by the compiler will modify the contract’s runtime code before it is returned by replacing all references to immutables by the values assigned to the them. This is important if you are comparing the runtime code generated by the compiler with the one actually stored in the blockchain.

contract One:
  const SOME_CONST: u256

  pub def __init__(val: u256):
    SOME_CONST = val

  pub def multiply_by_const(val: u256) -> u256:
    return val * SOME_CONST

Open question: would this actually need to be read/write through self.SOME_CONST because technically there's no reason to require self since we aren't reading from contract storage.

---

Do I have these three cases right? Maybe there's value to not use a different keyword for case 3? My immediate reaction would be that const is a good fit for all three of these cases but who knows.

[g-r-a-n-t]

1. A const outside of a contract. This type of constant can be used across multiple contracts. Each contract uses the data mechanism to store and load them.

It might also be possible to compile base type const values in-line as opposed to reading them from data, as a means to save gas. This would make compilation more complicated, tho, since const values set during initialization would need to be managed using the setimmutable function, which of course relies on data objects.

A const field of the contract that is assigned at declaration time. This type of constant can only be used within the contract it is defined...

What is meant by "declaration time"?

If the value is set in the the init function, then we would overwrite the compiled value during initialization. But if the value is not set in the init function, then we would leave it alone.

So the stages with regard to const values would be:

1. compile-time: module level and contract level consts are included in the bytecode (values are zeroed if there is no rhs)
2. initialization: assignments to const values overwrite what has been compiled in using setimmutable
3. runtime: at this point, there is no difference between a const value set during compile-time or initialization. it is also impossible to change a const value.

...Each contract uses the data mechanism to store and load them.

Yes, but to be more specific I'd say: Contracts use the setimmutable function to initialize const values, if they are changed in __init__. To load values, contracts use the data... functions.

would this actually need to be read/write through self.SOME_CONST because technically there's no reason to require self since we aren't reading from contract storage.

Hard to say, but I don't think whether or not it's held in contract storage is the deciding factor. I think it's more a question of what looks the best and is consistent with other rules, which requires some more thought.

Do I have these three cases right? Maybe there's value to not use a different keyword for case 3? My immediate reaction would be that const is a good fit for all three of these cases but who knows.

I think all three should have the same name. We've also discussed having an immut keyword for function parameters, so maybe we want to consider using that instead of const.

[g-r-a-n-t]

I think all three should have the same name. We've also discussed having an immut keyword for function parameters, so maybe we want to consider using that instead of const.

hmm.. or maybe the default mutability of a contract field should be immutable (aka const), which gets into the problem of not having a mut keyword yet.

[g-r-a-n-t]

would this actually need to be read/write through self.SOME_CONST because technically there's no reason to require self since we aren't reading from contract storage.

After thinking about it more, I believe the relevant question is, is this part of a contract instance? I believe it should be, as the following misleading_pure function would not really be pure:

contract Foo:
  const BAR: u8

  pub fn __init__(bar: u8):
    BAR = bar

  # return value of BAR (which takes no args) is not known at compile-time
  pub fn misleading_pure() -> u8:
    return BAR 

This on the other hand should be totally ok.

const BAR: u8 = 26

contract Foo:
  pub fn actual_pure() -> u8:
    return BAR 

[cburgdorf]

It might also be possible to compile base type const values in-line as opposed to reading them from data, as a means to save gas

Good point!

What is meant by "declaration time"?

I meant a const that is not assigned during __init__ but rather has a value assigned as part of its declaration:

contract One:
  const SOME_CONST: u256 = 1000

So the stages with regard to const values would be:

Yep, makes sense 👍

After thinking about it more, I believe the relevant question is, is this part of a contract instance?

Yes, after thinking about it a bit more I think accessing a const that is declared on the contract level without self would look really odd.

[cburgdorf]

Another question that appeared to me is how we want to deal with scoping rules. E.g. would this be legal:

contract Foo:
  const BAR: u256 = 10

  pub fn hello_world():
    let BAR: u256

What about this:

const BAR: u256 = 10

contract Foo:
  pub fn hello_world():
    let BAR: u256

My current thinking is that we should be strict and not allow shadowing constants with local variables since it can make contracts harder to audit.

[cburgdorf]

I removed the beta-required tag because we now support basic const on the module level which I think passes the minimal requirement.