-
Notifications
You must be signed in to change notification settings - Fork 7
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: auto-update ADDR in Makefile #63
base: main
Are you sure you want to change the base?
Conversation
contract/Makefile
Outdated
# ADDR=agoric1k78s7qz7rxy8afyjrqk3dntg8m83zaw3upe60p | ||
ADDR=agoric125d8s0tamx50gpqdd3m7f5gtkzkcnjs0ehsujr | ||
|
||
# fetch the address of the deployer account from the chain - you may need to adjust this to match your setup |
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.
Why would we need to adjust this?
Isn't the point of this pr to take care of that?
) | ||
|
||
$(info USERNAME is $(USERNAME)) | ||
$(info ADDR is $(ADDR)) |
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.
Please update (remove) the corresponding instructions in the README
What problem is this supposed to address? Is there some reason why we don't use a fixed address (from a fixed mnemonic) for user1? That's what we do when setting up the keyring in agoric-3-proposals and in getting started for dapp-offer-up and such:
|
I am not privy to the design decision to not use a fixed address, but current snippet saves me about ten seconds (calling the If we go for a fixed address, this will become irrelevant. If there is an issue, I can add a task there to remove/update this code with the fixed address when we close that.
|
@Jovonni I have successfully deployed contract with the using this PR with updated
|
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.
I'd prefer to see fixed addresses.
(I'm mostly submitting this review to remove this from my queue.)
This is same as #40 that was mistakenly merged to multichain-u17-with-icq instead of
main
.