-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add hardhat task to seed contract with data #83
Add hardhat task to seed contract with data #83
Conversation
// const signature = await signHre( | ||
// hre, | ||
// selectedArchaeologistSigners[i], | ||
// [doubleHashedShard, fakeShardTxId], | ||
// ["bytes32", "string"] | ||
// ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this signature block go bye-bye b/c there is the one above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah forgot to take this out
const minimumDiggingFee = 10; | ||
|
||
// 1 week | ||
const maximumRewrapInterval = 604800; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if this should be randomized (or able to be customized), so that the front-end would display some hidden archaeologists as well as some non-hidden. Not sure the best way to accomplish this to ensure you get some of each bucket (shown + hidden based on maxRewrapInterval)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can. I started out that way but then realized that the rewrap interval is part of message that each arch signs, so the arch's criteria need to match up with what's being passed into the createSarcophagus
function. So I set this as a constant to make things easier but we can certainly modify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original intent of this task was to create arch metrics, not to create archaeologists that can be viewed from the app. We can certainly add to this with a bit more work though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@knoll3 -- yeah, I think we can break this extra work out into a separate branch.
Since the place the reputation stats are going to mainly be viewed is on the front-end archaeologist multi-select, it's probably worth figuring out how to tie these registered archs to the arch service. I'll create a project card for this.
const archaeologistSigners: SignerWithAddress[] = []; | ||
for (let i = 0; i < count; i++) { | ||
const account = accounts[i]; | ||
const minimumDiggingFee = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be a randomized big number? That might help us test the digging fee filters on the front-end
https://ethereum.stackexchange.com/questions/97067/random-bignumber-in-ethers-js
example in that article looks like it lets you set a max value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same answer from the rewrap interval. I started out making it random then made it a constant to simplify the code.
const maximumRewrapInterval = 604800; | ||
const diggingFee = 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple comments on the register archaeologist about randomizing these values, and that would affect these -- the digging fees would need to be set to a per-archaeologist value (based on their minDiggingFee
) and the maximumRewrapInterval
may need to be set to the maximum of the maximumRewrapInterval
values (unless this value is automatically larger than the randomized vals, then it could be static)
await increaseTo(hre, resurrectionTime + 100); | ||
|
||
// Filter out the accuased sarcophagi for unwrapping | ||
const unaccusedSarcophagiData = sarcophagiData.filter( | ||
sarcophagus => !accusedSarcophagi.includes(sarcophagus.sarcoId) | ||
); | ||
|
||
// Unwrap some sarcohpagi | ||
await unwrapSarcophagi(hre, archaeologistSigners, unaccusedSarcophagiData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this automatically attempt to unwrap all unaccused sarcophagi?
We will probably want a way to have some sarcophagi that are not yet at the unwrap time yet.
Maybe an easy way would be to optionally skip this part of the script (to avoid fast-forwarding the block time). Another more involved version of this would be something like having 2 resurrection times, and fast-forwarding the block time between them.
await archaeologistFacet | ||
.connect(account) | ||
.registerArchaeologist( | ||
"some-peer-id", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanted to be able to hook up real archaeologists to these accounts (which may be useful), this could generate a real peerId. We would probably need to output the full peer Id JSON + signer private key (either via logs or write to a file) so archs could be hooked up with the registering account.
Otherwise we would need to disable the "check if online" portion of the front-end to see the archs, as they won't show up, which maybe what we want to do at this point to save time (we could discuss this).
@knoll3 -- most of my comments are related to modifying this to scaffold data for additional sarcophagi that are in the future (not just the past), and also so that the archaeologists generated could be attached to real online archs. Since this PR does a great job of setting up historical sarcophagi, these modifications could be done in a separate PR later, whenever testing these scenarios with larger sets of data would be useful. |
Add hardhat task to seed data to contract
Adds a hardhat task called
generate-history
that seeds the contract with archaeologists, accused sarcophagi, and successful sarcophagi.Config
Use the config file in
tasks/generate-history/config.ts
to define config values.Testing
--network localhost
.This will register archs, create sarcos, accuse archs on sarcos, and then unwrap sarcos.