Skip to content

Commit

Permalink
refactor: deprecate ConfigTimeConstants (FuelLabs#4298)
Browse files Browse the repository at this point in the history
## Description

Closes FuelLabs#3681 

I meant to tackle FuelLabs#3077 first, but tackling FuelLabs#3681 first cleans up a lot
of the code needed to make FuelLabs#3077 easier.

This PR is mostly cleanup:
- Completely removes usage of `ConfigTimeConstant` everywhere (in
forc-pkg, sway-core, sway-type)
- **(BREAKING)** Completely removes support for `[constants]` table in
the manifest, which was using the `ConfigTimeConstant` struct. This
change is breaking for app devs ([open
issue](FuelLabs/sway-applications#375)) and
from a [quick
search](https://github.com/FuelLabs/sway-applications/search?l=TOML&q=%5Bconstants%5D)
on our sway-applications repo, there's a number of manifests still using
the old version of configuration time constants.

Consequentially, the above change also allowed us to cut down on other
constructs like `ConstInjectMap` within forc-pkg, since the only thing
we need to inject now are const CONTRACT_IDs - this cuts down quite a
bit of now-redundant code.

- As a result of the removal of `ConfigTimeConstant`, a few tests using
that feature were also deleted.
- Also updated the `configurable` example to be known as
`configurable_constants` instead of `config_time_constant`.


**This PR does not:**
- Fix the manual creation of `CONTRACT_ID` const. To create the const,
we still undergo manual parsing and type-checking and finally inserting
into a `SymbolMap` manually. This will be handled in a separate PR to
tackle FuelLabs#3077

Other than removal of a lot of code to do with CTC, the main change here
that allows contract dependencies to still work here is instead of
relying on the `ConstInjectMap` which we used to inject both constants
and contract dependencies, there is now only a single contract ID value
that we require (this is abstracted as `ContractIdConst`) to inject into
the contract for tests.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
eightfilms authored Mar 17, 2023
1 parent f4cffba commit 032cd3b
Show file tree
Hide file tree
Showing 29 changed files with 131 additions and 318 deletions.
10 changes: 5 additions & 5 deletions docs/book/src/basics/variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,20 @@ let baz: bool = true;

If the value declared cannot be assigned to the declared type, there will be an error generated by the compiler.

## Configuration-time Constants
## Configurable Constants

Configuration-time constants (or configurable constants) are special constants that behave like regular constants in the sense that they cannot change during program execution, but they can be configured _after_ the Sway program has been built. The Rust and TS SDKs allow updating the values of these constants by injecting new values for them directly in the bytecode without having to build the program again. These are useful for contract factories and behave somewhat similarly to `immutable` variables from languages like Solidity.
Configurable constants are special constants that behave like regular constants in the sense that they cannot change during program execution, but they can be configured _after_ the Sway program has been built. The Rust and TS SDKs allow updating the values of these constants by injecting new values for them directly in the bytecode without having to build the program again. These are useful for contract factories and behave somewhat similarly to `immutable` variables from languages like Solidity.

Configuration-time constants are declared inside a `configurable` block and require a type ascription and an initializer as follows:
Configurable constants are declared inside a `configurable` block and require a type ascription and an initializer as follows:

```sway
{{#include ../../../../examples/config_time_constants/src/main.sw:configurable_block}}
{{#include ../../../../examples/configurable_constants/src/main.sw:configurable_block}}
```

At most one `configurable` block is allowed in a Sway project. Moreover, `configurable` blocks are not allowed in libraries.

Configurable constants can be read directly just like regular constants:

```sway
{{#include ../../../../examples/config_time_constants/src/main.sw:using_configurables}}
{{#include ../../../../examples/configurable_constants/src/main.sw:using_configurables}}
```
2 changes: 1 addition & 1 deletion examples/Forc.lock
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ source = 'member'
dependencies = ['std']

[[package]]
name = 'config_time_constants'
name = 'configurable_constants'
source = 'member'

[[package]]
Expand Down
2 changes: 1 addition & 1 deletion examples/Forc.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ members = [
"asm_return_tuple_pointer",
"break_and_continue",
"cei_analysis",
"config_time_constants",
"configurable_constants",
"counter",
"enums",
"fizzbuzz",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
implicit-std = false
license = "Apache-2.0"
name = "config_time_constants"
name = "configurable_constants"

[dependencies]
File renamed without changes.
6 changes: 0 additions & 6 deletions forc-pkg/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use std::{
};

use sway_core::{fuel_prelude::fuel_tx, language::parsed::TreeType, parse_tree_type, BuildTarget};
pub use sway_types::ConfigTimeConstant;
use sway_utils::constants;

/// The name of a workspace member package.
Expand Down Expand Up @@ -142,7 +141,6 @@ pub struct PackageManifest {
pub dependencies: Option<BTreeMap<String, Dependency>>,
pub patch: Option<BTreeMap<String, PatchMap>>,
/// A list of [configuration-time constants](https://github.com/FuelLabs/sway/issues/1498).
pub constants: Option<BTreeMap<String, ConfigTimeConstant>>,
pub build_target: Option<BTreeMap<String, BuildTarget>>,
build_profile: Option<BTreeMap<String, BuildProfile>>,
pub contract_dependencies: Option<BTreeMap<String, ContractDependency>>,
Expand Down Expand Up @@ -350,10 +348,6 @@ impl PackageManifestFile {
}
})
}
/// Getter for the config time constants on the manifest.
pub fn config_time_constants(&self) -> BTreeMap<String, ConfigTimeConstant> {
self.constants.as_ref().cloned().unwrap_or_default()
}

/// Returns the workspace manifest file if this `PackageManifestFile` is one of the members.
pub fn workspace(&self) -> Result<Option<WorkspaceManifestFile>> {
Expand Down
104 changes: 37 additions & 67 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use crate::{
lock::Lock,
manifest::{
BuildProfile, ConfigTimeConstant, Dependency, ManifestFile, MemberManifestFiles,
PackageManifestFile,
},
manifest::{BuildProfile, Dependency, ManifestFile, MemberManifestFiles, PackageManifestFile},
source::{self, Source},
CORE, PRELUDE, STD,
};
Expand All @@ -20,7 +17,7 @@ use petgraph::{
};
use serde::{Deserialize, Serialize};
use std::{
collections::{hash_map, BTreeMap, BTreeSet, HashMap, HashSet},
collections::{hash_map, BTreeSet, HashMap, HashSet},
fmt,
fs::{self, File},
hash::{Hash, Hasher},
Expand Down Expand Up @@ -270,8 +267,8 @@ pub struct MinifyOpts {
pub json_storage_slots: bool,
}

type ConstName = String;
type ConstInjectionMap = HashMap<Pinned, Vec<(ConstName, ConfigTimeConstant)>>;
/// Represents a compiled contract ID as a pub const in a contract.
type ContractIdConst = String;

/// The set of options provided to the `build` functions.
#[derive(Default)]
Expand All @@ -298,8 +295,6 @@ pub struct BuildOpts {
pub error_on_warnings: bool,
/// Include all test functions within the build.
pub tests: bool,
/// List of constants to inject for each package.
pub const_inject_map: ConstInjectionMap,
/// The set of options to filter by member project kind.
pub member_filter: MemberFilter,
}
Expand Down Expand Up @@ -396,14 +391,6 @@ impl BuildOpts {
..self
}
}

/// Return a `BuildOpts` with modified `injection_map` field.
pub fn const_injection_map(self, const_inject_map: ConstInjectionMap) -> Self {
Self {
const_inject_map,
..self
}
}
}

impl Edge {
Expand Down Expand Up @@ -1539,19 +1526,26 @@ pub const CONTRACT_ID_CONSTANT_NAME: &str = "CONTRACT_ID";
///
/// This function also ensures that if `std` exists in the graph,
/// then the std prelude will also be added.
///
/// `contract_id_value` should only be Some when producing the `dependency_namespace` for a contract with tests enabled.
/// This allows us to provide a contract's `CONTRACT_ID` constant to its own unit tests.
pub fn dependency_namespace(
lib_namespace_map: &HashMap<NodeIx, namespace::Module>,
compiled_contract_deps: &CompiledContractDeps,
graph: &Graph,
node: NodeIx,
constants: BTreeMap<String, ConfigTimeConstant>,
engines: Engines<'_>,
contract_id_value: Option<ContractIdConst>,
) -> Result<namespace::Module, vec1::Vec1<CompileError>> {
// TODO: Clean this up when config-time constants v1 are removed.
let node_idx = &graph[node];
let name = Some(Ident::new_no_span(node_idx.name.clone()));
let mut namespace =
namespace::Module::default_with_constants(engines, constants, name.clone())?;
let mut namespace = if let Some(contract_id_value) = contract_id_value {
namespace::Module::default_with_contract_id(engines, name.clone(), contract_id_value)?
} else {
namespace::Module::default()
};

namespace.is_external = true;
namespace.name = name;

Expand All @@ -1567,24 +1561,20 @@ pub fn dependency_namespace(
.cloned()
.expect("no namespace module"),
DepKind::Contract { salt } => {
let mut constants = BTreeMap::default();
let dep_contract_id = compiled_contract_deps
.get(&dep_node)
.map(|dep| contract_id(dep.bytecode.clone(), dep.storage_slots.clone(), &salt))
// On `check` we don't compile contracts, so we use a placeholder.
.unwrap_or_default();
// Construct namespace with contract id
let contract_id_value = format!("0x{dep_contract_id}");
let contract_id_constant = ConfigTimeConstant {
r#type: "b256".to_string(),
value: contract_id_value,
public: true,
};
constants.insert(CONTRACT_ID_CONSTANT_NAME.to_string(), contract_id_constant);
let node_idx = &graph[dep_node];
let name = Some(Ident::new_no_span(node_idx.name.clone()));
let mut ns =
namespace::Module::default_with_constants(engines, constants, name.clone())?;
let mut ns = namespace::Module::default_with_contract_id(
engines,
name.clone(),
contract_id_value,
)?;
ns.is_external = true;
ns.name = name;
ns
Expand Down Expand Up @@ -2025,7 +2015,6 @@ pub fn build_with_options(build_options: BuildOpts) -> Result<Built> {
binary_outfile,
debug_outfile,
pkg,
const_inject_map,
build_target,
member_filter,
..
Expand Down Expand Up @@ -2067,13 +2056,7 @@ pub fn build_with_options(build_options: BuildOpts) -> Result<Built> {
// Build it!
let mut built_workspace = Vec::new();
let build_start = std::time::Instant::now();
let built_packages = build(
&build_plan,
*build_target,
&build_profile,
&outputs,
const_inject_map,
)?;
let built_packages = build(&build_plan, *build_target, &build_profile, &outputs)?;
let output_dir = pkg.output_directory.as_ref().map(PathBuf::from);

let finished = ansi_term::Colour::Green.bold().paint("Finished");
Expand Down Expand Up @@ -2172,7 +2155,6 @@ pub fn build(
target: BuildTarget,
profile: &BuildProfile,
outputs: &HashSet<NodeIx>,
const_inject_map: &ConstInjectionMap,
) -> anyhow::Result<Vec<(NodeIx, BuiltPackage)>> {
let mut built_packages = Vec::new();

Expand All @@ -2185,7 +2167,10 @@ pub fn build(
let decl_engine = DeclEngine::default();
let engines = Engines::new(&type_engine, &decl_engine);
let include_tests = profile.include_tests;
let mut const_inject_map = const_inject_map.clone();

// This is the Contract ID of the current contract being compiled.
// We will need this for `forc test`.
let mut contract_id_value: Option<ContractIdConst> = None;

let mut lib_namespace_map = Default::default();
let mut compiled_contract_deps = HashMap::new();
Expand Down Expand Up @@ -2235,13 +2220,15 @@ pub fn build(
..profile.clone()
};

// `ContractIdConst` is a None here since we do not yet have a
// contract ID value at this point.
let dep_namespace = match dependency_namespace(
&lib_namespace_map,
&compiled_contract_deps,
plan.graph(),
node,
manifest.config_time_constants().clone(),
engines,
None,
) {
Ok(o) => o,
Err(errs) => return fail(&[], &errs),
Expand All @@ -2254,8 +2241,10 @@ pub fn build(
&mut source_map,
)?;

// If this contract is built because tests are enabled we need to insert CONTRACT_ID
// for the contract.
// If this contract is built because:
// 1) it is a contract dependency, or
// 2) tests are enabled,
// we need to insert its CONTRACT_ID into a map for later use.
if is_contract_dependency {
let compiled_contract_dep = CompiledContractDependency {
bytecode: compiled_without_tests.bytecode.bytes.clone(),
Expand All @@ -2269,34 +2258,14 @@ pub fn build(
compiled_without_tests.storage_slots,
&fuel_tx::Salt::zeroed(),
);
let contract_id_constant_name = CONTRACT_ID_CONSTANT_NAME.to_string();
let contract_id_value = format!("0x{contract_id}");
let contract_id_constant = ConfigTimeConstant {
r#type: "b256".to_string(),
value: contract_id_value.clone(),
public: true,
};
let constant_declarations = vec![(contract_id_constant_name, contract_id_constant)];
const_inject_map.insert(pkg.clone(), constant_declarations);
// We finally set the contract ID value here to use for compilation later if tests are enabled.
contract_id_value = Some(format!("0x{contract_id}"));
}
Some(compiled_without_tests.bytecode)
} else {
None
};

let constants = const_inject_map
.get(pkg)
.map(|injected_ctc| {
let mut constants = manifest.config_time_constants();
constants.extend(
injected_ctc
.iter()
.map(|(name, ctc)| (name.clone(), ctc.clone())),
);
constants
})
.unwrap_or_else(|| manifest.config_time_constants());

// Build all non member nodes with tests disabled by overriding the current profile.
let profile = if !plan.member_nodes().any(|member| member == node) {
BuildProfile {
Expand All @@ -2307,17 +2276,19 @@ pub fn build(
profile.clone()
};

// Note that the contract ID value here is only Some if tests are enabled.
let dep_namespace = match dependency_namespace(
&lib_namespace_map,
&compiled_contract_deps,
plan.graph(),
node,
constants,
engines,
contract_id_value.clone(),
) {
Ok(o) => o,
Err(errs) => return fail(&[], &errs),
};

let mut compiled = compile(
&descriptor,
&profile,
Expand Down Expand Up @@ -2506,14 +2477,13 @@ pub fn check(
for &node in plan.compilation_order.iter() {
let pkg = &plan.graph[node];
let manifest = &plan.manifest_map()[&pkg.id()];
let constants = manifest.config_time_constants();
let dep_namespace = dependency_namespace(
&lib_namespace_map,
&compiled_contract_deps,
&plan.graph,
node,
constants,
engines,
None,
)
.expect("failed to create dependency namespace");

Expand Down
2 changes: 0 additions & 2 deletions forc-plugins/forc-client/src/op/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ pub async fn deploy_pkg(
}

fn build_opts_from_cmd(cmd: &cmd::Deploy) -> pkg::BuildOpts {
let const_inject_map = std::collections::HashMap::new();
pkg::BuildOpts {
pkg: pkg::PkgOpts {
path: cmd.pkg.path.clone(),
Expand Down Expand Up @@ -177,7 +176,6 @@ fn build_opts_from_cmd(cmd: &cmd::Deploy) -> pkg::BuildOpts {
debug_outfile: cmd.build_output.debug_file.clone(),
build_target: BuildTarget::default(),
tests: false,
const_inject_map,
member_filter: pkg::MemberFilter::only_contracts(),
}
}
2 changes: 0 additions & 2 deletions forc-plugins/forc-client/src/op/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ async fn send_tx(
}

fn build_opts_from_cmd(cmd: &cmd::Run) -> pkg::BuildOpts {
let const_inject_map = std::collections::HashMap::new();
pkg::BuildOpts {
pkg: pkg::PkgOpts {
path: cmd.pkg.path.clone(),
Expand Down Expand Up @@ -173,7 +172,6 @@ fn build_opts_from_cmd(cmd: &cmd::Run) -> pkg::BuildOpts {
binary_outfile: cmd.build_output.bin_file.clone(),
debug_outfile: cmd.build_output.debug_file.clone(),
tests: false,
const_inject_map,
member_filter: pkg::MemberFilter::only_scripts(),
}
}
2 changes: 0 additions & 2 deletions forc-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,6 @@ impl<'a> PackageTests {
impl Opts {
/// Convert this set of test options into a set of build options.
pub fn into_build_opts(self) -> pkg::BuildOpts {
let const_inject_map = std::collections::HashMap::new();
pkg::BuildOpts {
pkg: self.pkg,
print: self.print,
Expand All @@ -360,7 +359,6 @@ impl Opts {
error_on_warnings: self.error_on_warnings,
time_phases: self.time_phases,
tests: true,
const_inject_map,
member_filter: Default::default(),
}
}
Expand Down
Loading

0 comments on commit 032cd3b

Please sign in to comment.