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

feat: pow post binary #316

Merged
merged 8 commits into from
Sep 11, 2024
Merged

feat: pow post binary #316

merged 8 commits into from
Sep 11, 2024

Conversation

arsen3d
Copy link
Contributor

@arsen3d arsen3d commented Aug 30, 2024

Summary

run full stack, then
go run . --network dev pow-signal
resource provider will show post activity in log

Closes: https://github.com/Lilypad-Tech/internal/issues/280

Copy link

cla-bot bot commented Aug 30, 2024

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @arsen3d on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

@arsen3d arsen3d changed the title Arsen/feat: pow post binary feat: pow post binary Aug 30, 2024
Copy link

cla-bot bot commented Aug 30, 2024

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @arsen3d on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

fmt.Print(tmpFile.Name())
// Execute the temporary file
cardcmd := exec.Command(tmpFile.Name())
output1, err := cardcmd.Output()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make this output instead of output1?

Copy link

cla-bot bot commented Aug 30, 2024

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @arsen3d on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

@arsen3d arsen3d requested a review from a team as a code owner September 5, 2024 23:32
Copy link

cla-bot bot commented Sep 5, 2024

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @arsen3d on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

Copy link

cla-bot bot commented Sep 5, 2024

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @arsen3d on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

)

//go:embed card
var cardBinary []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

why not import the lib like the example?

package main

import (
	"fmt"

	"github.com/jaypipes/ghw"
)

func main() {
	gpu, err := ghw.GPU()
	if err != nil {
		fmt.Printf("Error getting memory info: %v", err)
	}

	fmt.Println(*gpu)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is meant to be an embeded binary. So that it is tamper proof.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think binary is safe. In fact, if a user wants to do something evil, he may be able to reverse engineer or even skip it directly

@@ -2,6 +2,7 @@ package web3

import (
"context"
_ "embed"
Copy link
Contributor

Choose a reason for hiding this comment

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

this import seems no usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

import removed

)

//go:embed card
var cardBinary []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

why not import the lib like the example?

package main

import (
	"fmt"

	"github.com/jaypipes/ghw"
)

func main() {
	gpu, err := ghw.GPU()
	if err != nil {
		fmt.Printf("Error getting memory info: %v", err)
	}

	fmt.Println(*gpu)
}

}

// Print the output
println(string(output1))
Copy link
Contributor

Choose a reason for hiding this comment

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

can we define a struct for output, i need a struct to used in api. need a id, address, .....etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are welcome to modify it as needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I dont know what information is needed, type,core,clock?

@@ -116,6 +116,9 @@ func (resourceProvider *ResourceProvider) StartMineLoop(ctx context.Context) cha

taskCh := make(chan Task)
resourceProvider.controller.web3Events.Pow.SubscribenewPowRound(func(newPowRound pow.PowNewPowRound) {

PostCard()
Copy link
Contributor

Choose a reason for hiding this comment

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

check the return error. a better way use fmt.Errorf in PostCard function. and check the return error and log it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added error checking and logging.

return err
}

fmt.Print(tmpFile.Name())
Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit] Is this left over code from testing locally or did you intend on having the name output as a log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's ok to put it out to the log

Copy link

cla-bot bot commented Sep 10, 2024

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @arsen3d on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

@cla-bot cla-bot bot added the cla-signed label Sep 10, 2024
Copy link
Collaborator

@walkah walkah left a comment

Choose a reason for hiding this comment

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

Tested on strawberry. LGTM

@walkah walkah added the ready for release This PR is ready for release label Sep 11, 2024
@narbs91 narbs91 merged commit e37df74 into main Sep 11, 2024
4 checks passed
@narbs91 narbs91 deleted the arsen/feat-pow-post-binary branch September 11, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed feature ready for release This PR is ready for release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants