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

Improve random distribution #428

Merged
merged 2 commits into from
May 9, 2024
Merged

Conversation

tarakby
Copy link
Contributor

@tarakby tarakby commented May 8, 2024

New modulo argument in Cadence's revertibleRandom improves the output distribution to be uniform. Using the new argument is a better approach than computing modular reduction using the % operator, known to introduce the "modulo bias".
This PR improves calls to revertibleRandom for a better uniformity in :

Side change:

  • use a random number of 128 bits instead of two calls of 64 bits, now that revertibleRandom allows it

@tarakby tarakby requested a review from joshuahannan as a code owner May 8, 2024 18:48
@tarakby tarakby requested a review from jordanschalm May 8, 2024 18:49
@tarakby tarakby merged commit 7cecccd into stable-cadence May 9, 2024
2 checks passed
@joshuahannan joshuahannan deleted the tarak/rand-modulo-improv branch May 9, 2024 18:09
joshuahannan added a commit that referenced this pull request Jun 12, 2024
* update to view functions for stable cadence

* Update to newest Stable Cadence Preview (#371)

* update to view functions for stable cadence

* remove unreachable code

* update templates to Stable Cadence

* update to Stable Cadence preview 4

* update for stable cadence

* fix parse error

---------

Co-authored-by: Josh Hannan <[email protected]>
Co-authored-by: Bastian Müller <[email protected]>

* remove casting and fix conflicts

* integrate ft and nft stable cadence changes, use access, view, and entitlements

* use v2 ft and nft

* use string for nft import

* update token deps

* update token deps

* update nft dependecy

* update nft dep

* update ft dep

* add more view

* update FlowToken contract getter to explicitly use metadata views

* implement new standard methods

* update nft dependency

* update ft dep

* fix FungibleToken.Vault types and imports

* remove view from copy

* add correct vault type

* remove views and update balances

* use vault interface type

* remove restricted types

* remove AnyStruct

* remove type restrictions

* fix view modifiers in epoch contracts

* remove view for dkgCompleted

* remove non-view methods from pre-conditions

* use setter methods in QC

* remove pub from QC

* add fun keyword

* use setters for fields and use getBalance

* use view keyword

* remove restricted types from flow epoch

* use setters in FlowEpoch

* correct metadata types

* use UInt64 in NodeVersionBeacon

* dont use restricted types in locked tokens and staking collection

* make LockedTokens compatible with FungibleToken

* use entitlement for manager

* correct entitlement syntax

* fix entitlement issues with locked tokens

* fix entitlement issues with staking collection

* fix entitlement return types

* add withdrawable to machine account functions

* replace addresses and fix transactions and scripts

* fixes to epoch tests

* update to latest token standards and update locked tokens transactions

* Update contracts

* Update more contracts

* Update contracts

* Update contarcts and transactions

* go mod tidy

* Update flow-sdk

* Update more contracts

* Update ft/nft deps

* Generate assets

* Update more cadence codes

* Update staking collection

* Update go mods

* Fix typo

* Replace get()+borrow() with borrow()

* Refactor code

* Update go-sdk and flow-nft versions

* Fix publish_voter transaction

* Update to Cadence v1.0.0-preview.1

* Apply suggestions from code review

Co-authored-by: Bastian Müller <[email protected]>

* Refactor code

* fix and improve syntax

* use optional chaining and nil-coaelescing instead of force-unwrapping

* Update get_total_balance.cdc

* make sure epoch contracts work with capability controllers and entitlements

* update dependencies and add public burn tokens method

* remove view from getRandomSourceHistoryPage and use account.storage

* remove custom destructors and fix entitlement usage

* get all go tests passing for cadence 1.0

* make ci

* make ci

* update random source transactions

* update cli version for CI

* update go version

* Update Cadence version (#406)

* update Withdrawable and createEmptyVault

* update createEmpty in staking

* use Burner and update dependencies

* update to SDK v1.0.0-M1

* update NFT contracts, remove replace statements

* update to Cadence v1.0.0-M3

* update to Cadence v1.0.0-M3 and SDK v1.0.0-M1

* remove getBalance, add isAvailableToWithdraw

* update ft and nft deps

* update emulator dependencies and get tests passing

* use FungibleToken.Balance instead of FlowToken.Vault

* use correct public path types

* update imports

* go mod tidy

* add fungible token switchboard

* fix switchboard

* update error messages for ledger transactions

* add updated manifests with new account, FT, and NFT transactions and removed locked token staking txs

* update account transactions to use simple arguments

* update manifest to include both styles of token transfer transactions:

* update emulator dependency

* use string args for NFT contracts

* update collection machine account transactions to not use keylist entry

* Update contract getter methods to use env (#410)

* update import addresses to better format and use env

* update deps

* update go mod

* re-add lockedaccountinfo

* update ft and nft dependencies

* get address replacement working in tests and add event emission restrictions for FlowToken

* update to latest emulator, cadence, and token standards versions

* Update README

* Don't copy dictionary in getEpochMetadata (#413)

* Don't copy dictionary in getEpochMetadata

* update NFT contracts

* update ft dependency

* update token standard dependencies

* make ci

* fix algorithm typo and test names

* update nft transactions

* fix typos in transactions

* update nft dependencies

* Add entitlement for locked account creator (#423)

* update deps, add entitlement for locked account creator

* make ci

* update ft dependencies

* remove optional chaining in FlowStakingCollection

* remove optional chaining

* update nft imports and remove unnecessary Burner import from FlowToken

* update dependencies

* Improve random distribution (#428)

* better use of  revertible random with correct types

* improve random distribution

* address PR comments and add entitlements to LockedTokens.TokenManager

* remove UseTokens entitlement

* use view more and update nft deps

* make ci

* add view modifiers

* update dependencies

---------

Co-authored-by: Daniel Sainati <[email protected]>
Co-authored-by: Bastian Müller <[email protected]>
Co-authored-by: Supun Setunga <[email protected]>
Co-authored-by: Bastian Müller <[email protected]>
Co-authored-by: Janez Podhostnik <[email protected]>
Co-authored-by: Tarak Ben Youssef <[email protected]>
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.

3 participants