Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update node to version 6.13 #1732

Merged
merged 26 commits into from Apr 23, 2024
Merged

Conversation

vatanasov
Copy link
Contributor

The node now requires OTP 24, so the versions of elixir and erlang were also bumped (elixir: 1.13 -> 1.16, erlang: 23 -> 26).

Most of the changes in this PR are related to the fact, that tests are now sometimes executing in different order or different speed (probably because of optimizations) and also changes in how maps are ordered (which is now not-deterministic).

@vatanasov vatanasov self-assigned this Apr 18, 2024
def fetch_balances(state, contract_pk, top?) do
@spec fetch_balances(State.t(), State.t(), pubkey(), boolean()) ::
{:ok, amounts()} | {:error, Error.t()}
def fetch_balances(state, async_state, contract_pk, top?) do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change, and all that follows, where async_state is explicitly passed (and stored, in the case of AsyncStoreServer) are made so that the state can be reliably mocked and not relied on the global one.


:rocksdb.transaction_iterator(db_ref, transaction, cf_ref, read_options)
:rocksdb.transaction_iterator(transaction, cf_ref, read_options)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -152,7 +152,7 @@ defmodule AeMdwWeb.BlockchainSim do

{_prev_hash, mock_blocks, mock_txs, _max_height} =
blocks
|> Enum.reduce({@genesis_prev_hash, %{}, %{}, @initial_height}, fn
|> Enum.reduce({@genesis_prev_hash, %{}, [], @initial_height}, fn
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching them to Keyword instead of Map makes the order deterministic (some tests rely on that)

@@ -19,17 +19,17 @@ defmodule AeMdw.Db.AsyncStore do
@typep key() :: Database.key()
@typep record() :: Database.record()
@typep table() :: Database.table()
@opaque t() :: %__MODULE__{tid: :single_async_store}
@type t() :: %__MODULE__{tid: :single_async_store}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to expose the internal structure? I'd rather not

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise dialyzer didn't allow using pattern match on the struct itself (not only on the properties, probably because the struct is in a property) like in here: https://github.com/aeternity/ae_mdw/pull/1732/files#diff-50003680f6579cccc558e6fd7799dad10972701a70a1a307a18f977e94634e07R32 and several other places. I prefer having explicit matches, because this will make it fail sooner and more obvious, if something else gets passed, but I can change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep structures encapsulated in their own modules than exposing them for runtime checks. We try to avoid runtime errors and prioritize encapsulation and dialyzer (static) checks. If anyone tries sending anything other than an AsyncStore is going to get caught even before executing the app


dev_benefs =
for {protocol, _height} <- :aec_hard_forks.protocols(),
{pk, _} <- :aec_dev_reward.beneficiaries(protocol) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why credo didn't catch it, but can we name unused variables anyway?

.tool-versions Outdated Show resolved Hide resolved
@sborrazas sborrazas merged commit 0828e4b into master Apr 23, 2024
7 checks passed
@sborrazas sborrazas deleted the chore-use-node-version-6.13 branch April 23, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants