Add a package to run a script against all accounts #1434
Conversation
turbolent
requested review from
dsainati1,
robert-e-davidson3 and
janezpodhostnik
Codecov Report
@@ Coverage Diff @@
## master #1434 +/- ##
==========================================
- Coverage 74.41% 74.40% -0.01%
==========================================
Files 289 289
Lines 55671 55671
==========================================
- Hits 41426 41423 -3
- Misses 12751 12752 +1
- Partials 1494 1496 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit 1ef8460 Results
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentIndex uint | ||
| } | ||
|
|
||
| const endOfAccountsError = "storage used is not initialized or not initialized correctly" |
There was a problem hiding this comment.
Just a note: The address provider relies on a predictable error that happens when you to run getAccount(address).storageUsed for an address that dos not exist yet (is not initialised yet). There should probably be a better way to find out that the address does not exist within cadence.
There was a problem hiding this comment.
Yeah, when defining getAccount we didn't define the behaviour for accounts that do not exist. We should maybe define this better, e.g. abort execution, or allow it and add a field indicating if the account exists
| // We assume address #2 exists | ||
| lastAddressIndex, err := ap.getLastAddress(1, 2, true, addressExistsAtIndex) |
There was a problem hiding this comment.
Maybe short-circuit the check if address #2 doesn't exist?
There was a problem hiding this comment.
I just copied the internal code as is and didn't notice this.
@janezpodhostnik Do you know why address 2 is assumed to exist?
Maybe you @tehranifar?
There was a problem hiding this comment.
Seems like the code was refactored by @janezpodhostnik and @farbodg
The original version was meant to drive a script that operates on a range of accounts, e.g. 30, 31, 32. If one of those accounts doesn't exist the script panics so the code had to try with 30, 31 and then 30 to make sure it's capturing the end range correctly. The new version is probably achieving the same.
There was a problem hiding this comment.
I added that assumption (that account 2 exists) because all the networks the program would be run on had account 2.
During bootstrapping we create at least 4 accounts.
If this is a problematic assumption, the code just needs to test that address no. 2 exists, and if it doesn't address no. 1 is the last address.
| // 4. (6,8): check address 7 address exists so next pair is (7,8) | ||
| // 5. (7,8): check address (8 - 7) / 2 = 7 ... ok already checked so this is the last existing address | ||
| // | ||
| func (p *AddressProvider) getLastAddress( |
There was a problem hiding this comment.
Go really needs to get fast generics so this kind of thing can be generalized :/
There was a problem hiding this comment.
Hmm, how would generics help here, the types are always constant, no?
|
|
||
| module github.com/onflow/cadence/tools/batch-script | ||
|
|
||
| go 1.17 |
There was a problem hiding this comment.
other modules still seem to use 1.16
Description
Add a package that allows running a script against all accounts on a network.
Includes a tool to fetch all contracts and write them into a CSV file.
Code is taken from internal tool authored by @tehranifar, @farbodg, and @janezpodhostnik 🙏
masterbranchFiles changedin the Github PR explorer